1

I just started programming after taking a long break and am running into a little issue.

I am using VS2013 Desktop and am creating this as a GUI program.

My problem is that I created a working random number generator when the method used to call the logic method runs once. The number(s) gets generated, text updated and all is good. When it goes into a loop, it doesn't update the text property of the object I'm modifying until it finishes the entire loop or gets broken. The program basically hangs when I run it when the loop gets executed and I have to force it to close.

At the moment I would like to set the generator to run infinitely in the background until I press another button to stop it.

I am new to programming and this probably has all sorts of issues with it so I would be grateful for any feedback on structure and other practices if anything is out of order as well.

Here is the code:

Form1.cs

    // Global
    bool boolLooper;

    // Setting up the random number generator
    private string RandomNumber()
    {
        RandomNumber rndNumber = new RandomNumber();

        string strRandNumber = Convert.ToString(rndNumber.RandomInt(1000, 9999999));

        return strRandNumber;
    }

    // TEST - used in buttonclick event
    private void TextUpdates()
    {
        while (BoolLooper == true)
        {
            txtID1.Text = RandomNumber();
            //txtName1.Text = RandomNumber();
            //txtSize1.Text = RandomNumber();
            //txtKey1.Text = RandomNumber();
            //txtType1.Text = RandomNumber();
        }
    }

    //-----------------------------
    // Form - Button Clicks
    //-----------------------------

    // Button - Activate
    private void btnActivate_Click(object sender, EventArgs e)
    {
        BoolLooper = true;

        TextUpdates();

        //// Update text once
        //txtID1.Text = RandomNumber();
        //txtName1.Text = RandomNumber();
        //txtSize1.Text = RandomNumber();
        //txtKey1.Text = RandomNumber();
        //txtType1.Text = RandomNumber();

    }

    // Button - Stop/Deactivate
    private void btnDeactivate_Click(object sender, EventArgs e)
    {
        BoolLooper = false;
    }

    //-----------------------------
    // Properties
    //-----------------------------

    public bool BoolLooper
    {
        get { return boolLooper; }
        set { boolLooper = value; }
    }

RandomNumber.cs

    private static readonly Random intRandom = new Random();
    private static readonly object syncLock = new object();

    public int RandomInt(int minNum, int maxNum)
    {
        lock (syncLock)
        {   
            // synchronize
            return intRandom.Next(minNum, maxNum);
        }
    }

For the RandomNumber class, I found a great post on this site found here which I will give credit to it's author: https://stackoverflow.com/a/768001

Community
  • 1
  • 1
HaakonXCI
  • 19
  • 2
  • 1
    You need to look into threading and delegates. It doesnt update cause you are running on the UI thread so once its finished it redrawns the window and you see the last update. – Sorceri Sep 16 '14 at 13:43
  • This is normal behavior when you do work on the UI thread. You probably want a [background worker](http://msdn.microsoft.com/en-us/library/system.componentmodel.backgroundworker(v=vs.110).aspx) to make the changes. – crashmstr Sep 16 '14 at 13:44
  • Please, use `System.Windows.Timer` or `System.Timers.Timer` with `Invoke` method and common Random class. You have no multithreading here and you shouldn't use synchronization for this task. – Alex Zhukovskiy Sep 16 '14 at 13:46

5 Answers5

2

You're running this code on the same thread as the UI. Since it's single-threaded, the UI can't respond because it's busy running your loop. You'll want to off-load this to a separate thread or in some way as a separate asynchronous operation. That thread/operation would then just need to tell the UI of updates when it has them.

A simple example of this would be to use a BackgroundWorker object.

Note in the example on that page where the BackgroundWorker exposes an event which can be used to update UI elements:

private void backgroundWorker1_ProgressChanged(object sender, ProgressChangedEventArgs e)
{
    resultLabel.Text = (e.ProgressPercentage.ToString() + "%");
}

There are other possible approaches as well. You could create a Thread manually and try to synchronize it manually, but that comes with other potential issues as well. And there really isn't a need to get that complex here.

Do you need the TextBox to be constantly updating? Or just updating every once in a while? If there's some discernible time period between updates (one second?) then you can use a Timer to schedule the code to take place periodically. The structure is similar to the BackgroundWorker in that there's an event exposed which would be used to update the UI.

David
  • 208,112
  • 36
  • 198
  • 279
  • 8
    Every time you call `DoEvents`, I kill a kitten. Seriously: don't do that. That is a horrible kludge. – Marc Gravell Sep 16 '14 at 13:48
  • `DoEvents` is ugly and should never be used in production code, BUT, it can be useful for debugging. Please don't kill kittens. They didn't invent `DoEvents`. – David Crowell Sep 16 '14 at 13:49
  • 1
    @DavidCrowell fortunately they were only virtual kittens – Marc Gravell Sep 16 '14 at 13:50
  • @MarcGravell: Ya, I thought about it for a bit and decided to remove it entirely from the answer. It was really only there for "completeness", but the more I think about it the more we could use less of `Application.DoEvents()` in the world. – David Sep 16 '14 at 13:58
  • > Do you need the TextBox to be constantly updating? Yes, mainly for aesthetics and for some practice. I will look into that BackgroundWorker. Will update later with results. Thank you for your response. – HaakonXCI Sep 16 '14 at 14:04
1

All your code is being executed on the UI thread. So you're stuck in your while loop, and the form isn't responding to the button click (which sets your while loop flag back to false). This is what we call a blocking call. It's blocking the UI from continuing.

Typically in situations like this, you would want to look into threading. However, based on your code. I'd look into a timer, and have it tick every second or so. They're very easy to implement and you can remove the complexity of your while loop and just execute the random number generation and the assigning it to your UI controls. (This also makes it so that you don't have to marshal from a background thread back onto your UI thread.)

For more information on a timer: System.Windows.Forms.Timer

Cameron
  • 2,574
  • 22
  • 37
0

You basically need to run each call to generate a new number asynchronously. Using the .NET Framework, there are several ways to achieve that, but I prefer to use the Task class. You could do something like this:

public Task RunAsynchronously(Action method)
{
    return Task.Factory.StartNew(method);
}

...

RunAsynchronously(() => MethodThatGeneratesRandomNumber());

Each time this is called, the method execution will run asynchronously.

Sheridan
  • 68,826
  • 24
  • 143
  • 183
0

You are creating new instance of a RandomNumber class each time. Just make it a member of your class. Like :

// Global
bool boolLooper;
//Random number generator
RandomNumber rndNumber = new RandomNumber();

and don't need to create new instance in method RandomNumber , just change it to this:

private string RandomNumber()
{   
    string strRandNumber = Convert.ToString(rndNumber.RandomInt(1000, 9999999));

    return strRandNumber;
}

UPDATE: I've read a bit about Application.DoEvents() after comment, so use Invokes, await calls of Tasks, others, but not this.

prvit
  • 956
  • 3
  • 9
  • 22
  • Application.DoEvents() = EVIL!!!! if your threadings are done correctly you shouldn't need to call it to update you UI. – Mo Patel Sep 16 '14 at 14:04
  • @MPatel Well, you are absolutely correct, shame on me for recommending using it. I changed my answer :-) – prvit Sep 16 '14 at 14:12
0

if you are using .NET 4.5, update the TextUpdates method to use the async/await call like in the example below

private async void TextUpdates()
{
    await Task.Run(() =>
    {
        while (BoolLooper)
        {
            txtID1.Invoke((MethodInvoker)(() => txtID1.Text = RandomNumber()));
            //txtName1.Text = RandomNumber();
            //txtSize1.Text = RandomNumber();
            //txtKey1.Text = RandomNumber();
            //txtType1.Text = RandomNumber();
        }
    });
}
Mo Patel
  • 2,321
  • 4
  • 22
  • 37