Descending timer problem

[aminit]

Hello all:

I was developed a simple descending timer , the problem was while it's execute and after I entered the value in the textbox ,, this message appear:

(((Ontimer(): cross thread operation not valid "textbox1" accessed from the thread other than the thread was crated on )))

on timer is a function I made it for intialize the timer
and this is my code

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Timers;
using System.Reflection;

namespace NoorHSProject2008
{
    public partial class Form1 : Form
    {


        private System.Timers.Timer timerClock = new System.Timers.Timer();
        private int clockTime = 0;
        private int alarmTime = 0;



        public Form1()
        {
            InitializeComponent();
            InitializeTimer();
            
        }
        private void inputToSeconds(string timerInput)
        {
            try
            {
                string[] timeArray = new string[3];
                int minutes = 0;
                int hours = 0;
                int seconds = 0;
                int occurence = 0;
                int length = 0;

                occurence = timerInput.LastIndexOf(":");
                length = timerInput.Length;
                if (occurence == -1 || length != 8)
                {
                    MessageBox.Show("Invalid Time Format.");

                }
                else
                {
                    timeArray = timerInput.Split(':');

                    seconds = Convert.ToInt32(timeArray[2]);
                    minutes = Convert.ToInt32(timeArray[1]);
                    hours = Convert.ToInt32(timeArray[0]);

                    this.alarmTime += seconds;
                    this.alarmTime += minutes * 60;
                    this.alarmTime += (hours * 60) * 60;
                }
            }
            catch (Exception e)
            {
                MessageBox.Show("inputToSeconds(): " + e.Message);
            }
        }

        public void InitializeTimer()
        {
            timerClock.Elapsed += new ElapsedEventHandler(OnTimer);
            timerClock.Interval = 1000;
            timerClock.Enabled = true;
        }

        private void button1_Click(object sender, EventArgs e)
        {
            clockTime = 0;
            inputToSeconds(textBox1.Text);
       
        }
        public void OnTimer(object sender,ElapsedEventArgs e)
        {
           try{
               clockTime++;
               int countdown = alarmTime-clockTime;
               if (alarmTime != 0)
                    textBox1.Text = secondsToTime(countdown);
               if(clockTime ==alarmTime )
				{  
                   textBox1.Text ="00:00:00";
                    Form2 b = new Form2();
                    b.Show();
                    MessageBox.Show("Time is over");
            }
           }
                catch( Exception ex)
			{
				MessageBox.Show("OnTimer(): " + ex.Message );
			}		
			
        
        }

        

        public string secondsToTime(int seconds)
        {
            int minutes = 0;
            int hours = 0;

            while (seconds >= 60)
            {
                minutes += 1;
                seconds -= 60;
            }
            while (minutes >= 60)
            {
                hours += 1;
                minutes -= 60;
            }

            string strHours = hours.ToString();
            string strMinutes = minutes.ToString();
            string strSeconds = seconds.ToString();

            if (strHours.Length < 2) strHours = "0" + strHours;
            if (strMinutes.Length < 2) strMinutes = "0" + strMinutes;
            if (strSeconds.Length < 2) strSeconds = "0" + strSeconds;

            return strHours + ":" + strMinutes + ":" + strSeconds;
        }

    }

So where is the problem???

[Jugortha]

you have to define a string variable in witch you put the the text1.Text content, beacause the using it in the button1_click and OnTimer methods make your code thread unsafe,

string myString = "";
        private void button1_Click(object sender, EventArgs e)
        {
            clockTime = 0;
            myString = textBox1.Text;
            inputToSeconds(myString);
        }



        public void OnTimer(object sender, System.Timers.ElapsedEventArgs e)
        {
            try
            {
                clockTime++;
                int countdown = alarmTime - clockTime;
                if (alarmTime != 0)
                    myString = secondsToTime(countdown);
                if (clockTime == alarmTime)
                {
                    myString = "00:00:00";
                    Form2 b = new Form2();
                    b.Show();
                    MessageBox.Show("Time is over");
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show("OnTimer(): " + ex.Message);
            }


        }

[aminit]

Hello :

Thanks a lot for your answer, but while I run the program and after I input the value , the program dose nothing ??Can you please upload your project here so I can see where are the errors please ???

Thanks in advance.....

[Jugortha]

I advise you remodel the program try to write a program as simpliest as possible to avoid problems

[JerryShaw]

AMINIT,

This looks like the same program you sent to me earlier this week that I fixed for you.
Did you not get the zip file I sent back ?
The problem is the way you declare the out of class timer. The way it is defined, means it will run in its own thread. Threads can not adjust non-thread safe components in other threads (like this main thread).

private System.Timers.Timer timerClock = new System.Timers.Timer();

Above is the line that is causing your error.
Comment out that line, drag a timer component onto the form, and re-wire it to use the onTick event instead of the onTimer event (different args).

If you need me to re-post the program for you let me know. Otherwise please mark this post as solved.

Thanks,
Jerry

[_r0ckbaer]

The problem is the way you declare the out of class timer. The way it is defined, means it will run in its own thread.

What?:D That timer is certainly NOT out of class.

Threads can not adjust non-thread safe components in other threads (like this main thread).

That's completely wrong. What do you think methods like Control.BeginInvoke() are there for? It's absolutely possible to use UI controls in a non-UI thread.

private System.Timers.Timer timerClock = new System.Timers.Timer();

Above is the line that is causing your error.
Comment out that line, drag a timer component onto the form, and re-wire it to use the onTick event instead of the onTimer event (different args).

Wrong again. That line has absolutely nothing to do with the problem he's facing.
NB: timerClock could be marked as read-only, but ok, that's just a matter of personal taste.

Otherwise please mark this post as solved.

Well, what about marking it as solved now?:P Altough that code could be done in a much shorter way using the methods of eg. the datetime class

Here's the corrected version (only relevant code is posted):

private delegate void SetTextBoxTextHandler(string text);
        private void SetTextBoxt(string  text)
        {
            textBox1.Text = text;
        }

        public void OnTimer(object sender, ElapsedEventArgs e)
        {
            try
            {
                clockTime++;
                int countdown = alarmTime - clockTime;
                if (alarmTime != 0)
                {
                    string formattedTime = secondsToTime(countdown);
                    if (textBox1.InvokeRequired)
                    {
                        textBox1.BeginInvoke(new SetTextBoxTextHandler(SetTextBoxt), new object[]{formattedTime});
                    }
                    else textBox1.Text = formattedTime;
                }
                    
                if (clockTime == alarmTime)
                {
                    timerClock.Enabled = false;

                    if (textBox1.InvokeRequired)
                    {
                        textBox1.BeginInvoke(new SetTextBoxTextHandler(SetTextBoxt), new object[] { "00:00:00" });
                    }
                    else textBox1.Text = "00:00:00";
                    Form2 b = new Form2();
                    b.ShowDialog();
                    MessageBox.Show("Time is over");
                   
                }
            }
            catch (Exception ex)
            {
                MessageBox.Show("OnTimer(): " + ex.Message);
            }
        }

[JerryShaw]

Rockbaer,

Obviously you misread. The Timer class he is using will issue a cross-thread error because it is running in its own thread (nature of that timer class). That is why in your code response, you are using a delegate where he is not. Your delegate allows the code to work with components in the main thread.

The TimerClock class does not have a readonly attribute, so you are wrong in that appraisal.
The use of this TimerClock class IS the problem if the coder does not go through the process of creating delegates, and invoke the delegate when touching main thread non-thread safe components, as is quite evident in the cross thread error he received.

All that being said, introducing threading techniques at his level is not the right approach. Using a simple timer gets the job done. In fact, you built on his example code allowing the timer to control other objects in the main class, and then making two delegate calls to account for the non-thread safe textbox. IOW you are just perpetuating the use of an errant design, and probably doing more harm to his learning experience. If you want to give him a lesson in using threads, then provide him with a total re-write of the project, and explain the delegate and invoke techniques so he knows why and how they should be used.

If I were to write this program, I wouldn't be using a timer at all. I prefer using theads to get the granular control I desire. The coding techniques in his example code could be vastly improved. There are much better ways of handling time decoding. But that was not the problem he presented. The entire project could be cut in half using better techniques, but again, that was not the problem he wanted solved.

Giving a man a fish feeds him for a day. Teaching him how to fish, feeds him for life.

// Jerry

[JerryShaw]

AMINIT,

If your project requires that you "must" use the System.Timer.Timer class, then you can set the timer's SynchronizingObject property to the form or component to take care of that cross-threading error. That will also eleveate the issue of invoking delegates.

From MSDN:::

If you use the Timer with a user interface element, such as a form
or control, assign the form or control that contains the Timer to 
the SynchronizingObject property, so that the event is marshaled to 
the user interface thread.

:::

This type of timer runs in its own thread. If you drop a timer component onto your form, then it will automatically set the SynchronizingObject property for you.

BTW: Since _rockbaer opened the topic of using threaded techniques, I re-wrote your project (inluding the save controls to text methods you requested) using a thread and no timers. The code length was chopped to less than 50% of what you currently have. If you are interested, and ready to delve into threads, I can send you the updated project. It will also show you how you can use the DateTime and TimeSpan classes instead of all those time decoding methods.

//Jerry