0

I'm writing a program to visualize different sorting algorithms. I'm using Winforms and using the form's paint even to update the graphs. The problem that I'm having is that I can't simply write a method that sorts the array because the form only gets updated when the method is completed. I tried using a timer as a for loop with global variables, and calling Refresh() but it makes my code a lot less reusable and looks ugly.
How can I fix this issue?
Edit: To be clear, the form isn't locking up, the form just isn't repainting itself.

Edit2: Here is where I'm currently calling my sorting methods. I had to replace the outer for loop that you normally use to sort arrays, with global variables

        private void Timer_Tick(object sender, EventArgs e)
        {
        switch (sortMethod)
        {
            case "Selection Sort":
                color = Sort.SelectionSort(color, ref i);
                break;
            case "Bubble Sort":
                color = Sort.BubbleSort(color, ref i);
                break;
            case "Cocktail Shaker Sort":
                bool sorted = false;
                color = Sort.CocktailShakerShort(color, ref sorted);
                break;
            case "Gnome Sort":
                if (pos < color.Length)
                {
                    color = Sort.GnomeSort(color, ref pos);
                }
                break;
            default:
                timer.Enabled = false;
                break;
        }
        Invalidate();
    }
Keegan Fargher
  • 3
  • 2
  • 5
  • 1
    This sounds like a good fit for the code review stack exchange https://codereview.stackexchange.com Also make sure to include your code – Patrick Hollweck Aug 28 '18 at 19:07
  • 1
    We need to see the code that isn't working. – LarsTech Aug 28 '18 at 19:08
  • 1
    Let me see if I understand the problem. You wish to display the *workings* of particular sorting algorithms as they work. Is it that every "timer tick" you want to *inspect* the current state of the algorithm and display how much progress it has made so far? Or do you want the algorithm to *inform the UI* every time it makes a change and let the UI decide whether or not to display the results? – Eric Lippert Aug 28 '18 at 19:21
  • Try using multi-threading and delegates, as explained here: https://msdn.microsoft.com/en-us/library/ms951089.aspx – Ruud Helderman Aug 28 '18 at 19:22
  • 1
    Because this is the sort of decision that *massively* impacts the design of the program. Basically, are you *pulling* information and displaying it when you want to, or is the information changing and *pushing* information about the change to the display code? – Eric Lippert Aug 28 '18 at 19:22
  • 2
    Do not use multithreading and delegates; that is opening yourself up to a world of pain. Particularly if you are a beginner programmer. **Ideally you only create threads when the work is CPU bound and there is a free CPU to dedicate the thread to**. Your problem is a UI problem. **Use non-thread-based asynchrony to build reactive user interfaces. Do not use multiple threads.** – Eric Lippert Aug 28 '18 at 19:23
  • But regardless, it sounds like what you want is a method that can do some work, suspend its execution, display information about the work, and then resume execution. Such a method is called a *coroutine*, and C# supports coroutines in two main forms, the *pull* form of iterator blocks, and the *push* form of async blocks. That's why I asked the question before about whether you are conceiving this as a pull or push problem. – Eric Lippert Aug 28 '18 at 19:25
  • @EricLippert I ideally want to put my sort methods into another class and update the form on demand to demonstrate the progress of the sorting method. – Keegan Fargher Aug 28 '18 at 19:31
  • @EricLippert You have my attention. The article I linked to is old, but [this article](https://learn.microsoft.com/en-us/windows/uwp/debug-test-perf/keep-the-ui-thread-responsive) timestamped 2017 still recommends: "In cases where a non-trivial amount of work needs to be performed, schedule it on a background thread and return." I must admit I never gave Winforms much attention, so I may have missed the memo explaining the dangers of multithreading. Any articles you can recommend? – Ruud Helderman Aug 28 '18 at 20:25
  • 1
    @RuudHelderman: That section of the document says that the scenario in which it is appropriate to move work onto a background thread is *when there is an expensive CPU-bound computation that does not have to update the UI*. The dangers are many; we've already noted that multithreading works poorly with user interfaces. But the bigger danger is shared memory. **C# does not guarantee that a single consistent ordering of side effects will be observed by multiple threads**. – Eric Lippert Aug 28 '18 at 20:55
  • @EricLippert Considering OP's remark that "the form isn't locking up", I'm probably just overestimating the amount of data OP is sorting. But I'm curious: how could sorting a _big_ data set in memory (and visualizing it in real time, as requested by OP) ever run on the UI thread without freezing up the UI? `Application.DoEvents` comes to mind, but isn't that just another gun for beginner programmers to shoot themselves in the foot (as explained [here](https://stackoverflow.com/questions/5181777/use-of-application-doevents))? Or is Winforms just not fit for this scenario? – Ruud Helderman Aug 29 '18 at 10:15
  • 1
    @RuudHelderman: That's a *great question*. My comments here are trying to elicidate if `await Task.Yield();` in the inner loop of the sort is appropriate. It's really important to understand why `DoEvents` is poison and `await Task.Yield();` is awesome. They appear to do the same thing, but the *way* they do it is subtly different. `DoEvents` *synchronously runs events from the message queue*. `Yield` *puts an event into the queue and returns to the caller*. If it is not clear why the former is super dangerous and the latter is awesome, well, this comment is too short to explain. :-) – Eric Lippert Aug 29 '18 at 16:55
  • @EricLippert Sounds very promising. But according to [this](https://stackoverflow.com/questions/23431595/task-yield-real-usages), `await Task.Yield()` may fail to keep the UI responsive, as the continuation message has a higher priority than user input, WM_PAINT and WM_TIMER messages. – Ruud Helderman Aug 29 '18 at 21:43
  • @RuudHelderman: **Fascinating**. I had no idea that was the case, and it at first hearing sounds like a design flaw. I expect yield to yield. – Eric Lippert Aug 29 '18 at 21:45

0 Answers0