2

I am new to coding and are having trouble.

I want to use a timer that will start upon pressing ScatterMode tab, it will start counting 4sec before running "Dosomething" function. This cycle will repeat itself until i decide to stop the program. But the problem i get is, this code only run correctly for like 2loop, after that the timer sort of go crazy LOL.

     System.Windows.Forms.Timer timer = new System.Windows.Forms.Timer();

    //ScatterMode Tab
    private void Scatter_modeToolStripMenuItem_Click(object sender, EventArgs e)
    {
            timer.Interval = 4000;
            timer.Enabled = true;
            timer.Tick += new EventHandler(Dosomething);
            timer.Start();
    }

    private void Dosomething (object sender, EventArgs e)
    {
        timer.Stop();
        timer.Enabled = false;

        Grab.buffer(out buffer, out status, 6000);
        Scatter_mode(buffer);
        pictureBox1.Refresh();

        int done_grab = 1;

        if (doneGrab == 1)
        {
            timer.Interval = 4000;
            timer.Enabled = true;
            timer.Tick += new EventHandler(Scatter_modeToolStripMenuItem_Click);
            timer.Start();
            done_grab = 0;
        }
    }
4
  • What does "sort of go crazy" mean? Commented Dec 20, 2017 at 9:48
  • I'll recommend you not using Forms.Timer because it usually locks the UI, and for "it will start counting 4sec before running "Dosomething", you can use a delay, or pause the threat Commented Dec 20, 2017 at 9:54
  • @Ferus7 I think thread pausing will be far worse/more problematic than using Timer - if the OP uses the same thread as is handling the window messages, then sleeping it for 4s will hang the application visually, and depending on how the looping is handled, it may never get to return to the messaging queue (hanging the app from user input forever). If another thread is used, then Invoke will be required because windows controls must only be accessed from the thread they were created on. This adds an unnecessary level of complexity. Timer is the best simple solution here Commented Dec 20, 2017 at 10:13
  • @ Caius Jard IMHO UI Timers must be avoided, if the thread is busy, it will take more than 4 secs to call DoSomething using a separate thread just needs a BeginInvoke to call the UI thread, and is a good practice to start with, but as I said, is only my recommendation. The thread delaying, ofc need multithreading, btw, It was nice to talk to you :), thank you so much Commented Dec 20, 2017 at 10:18

1 Answer 1

6

Adding a new event handler to a timer, to handle its tick event, inside the handler for the tick event will indeed cause the timer to go "crazy". Every time the timer raises its event, another event handler (that responds to events raised) will be added. This means the next time the timer ticks, the event code will run twice. Two new event handlers will be added. Next time the timer ticks, the code will run 4 times. 4 event handlers will be added ... and so on

Remove this line from your code:

timer.Tick += new EventHandler(Scatter_modeToolStripMenuItem_Click);

And move this line into your form's constructor:

timer.Tick += new EventHandler(Dosomething);

You only want to wire this event handler up once. Every time the timer's interval elapses, the code will run, once :)

I'll also do a bit of a peer review of your code, see the comments:

 System.Windows.Forms.Timer timer = new System.Windows.Forms.Timer();

//ScatterMode Tab
private void Scatter_modeToolStripMenuItem_Click(object sender, EventArgs e)
{
        timer.Interval = 4000; //can go in the constructor also; don't need to set repeatedly
        timer.Enabled = true;
        timer.Tick += new EventHandler(Dosomething); //move to constructor
        timer.Start(); //this isn't needed - you already Enabled the timer, which started it
}

private void Dosomething (object sender, EventArgs e)
{
    timer.Stop();          //use this
    timer.Enabled = false; //or this. It's not required to do both

    Grab.buffer(out buffer, out status, 6000);  //if these lines crash then your timer will
    Scatter_mode(buffer);                       //only restart if the toolstripmenuitemclick
    pictureBox1.Refresh();                      //above runs.. is it what you wanted?

    int done_grab = 1; //not needed

    if (doneGrab == 1) //this will always evaluate to true, it is not needed
    {
        timer.Interval = 4000; //the interval is already 4000, not needed
        timer.Enabled = true; //careful; your timer may stop forever if the code above crashes
        timer.Tick += new EventHandler(Scatter_modeToolStripMenuItem_Click); //remove
        timer.Start(); //not needed
        done_grab = 0; //not needed
    }
}
Sign up to request clarification or add additional context in comments.

4 Comments

OMG I LOVE YOU THANKS! I STUCK AT THIS THING FOR LIKE HOURS
No probs. If you consider an answer to be the right one, click the tick symbol on the left of it :)
Let's say i have another mode tab that requires timer too, so do i just add Tick += new EventHandler(another_mode); to the constructor? Will that cause the EventHandler to go crazy again?
No, that code will also only run once per elapsing of the timer, and you can use the same timer. It might not be guaranteed which order the codes will be called in, so don't make them dependent on each other/don't make them demand a specific order. The problem was purely that attaching another event handler inside the event handler made the code run 1, then 2, 4, 8, 16, 32, 64 .. 65536. times. By the 32nd tick, the code would be running 4294967296 times.. Pretty sure it would eventually fall apart

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.