patterncsharpMinor
Timer Tick Event only to execute at specific time
Viewed 0 times
timetimerspecifictickonlyexecuteevent
Problem
I have a timer that is only supposed to tick between x and x on weekdays.
I have used the following implementation.
Is this the way to go?
I have used the following implementation.
void tmrMain_Tick(object sender, EventArgs e)
{
if (!(DateTime.Now.DayOfWeek == DayOfWeek.Saturday || DateTime.Now.DayOfWeek == DayOfWeek.Sunday))
{
if ((IsTimeOfDayBetween(DateTime.Now, new TimeSpan(8, 0, 0), new TimeSpan(18, 0, 0))))
{
if (pnlAssembly.Visible == true)
{
cmdShed.PerformClick();
}
else if (pnlShed.Visible == true)
{
cmdServices.PerformClick();
}
else if (pnlWeb.Visible == true)
{
cmdAssembly.PerformClick();
}
}
}
}
static bool IsTimeOfDayBetween(DateTime time,
TimeSpan startTime, TimeSpan endTime)
{
if (endTime == startTime)
{
return true;
}
else if (endTime = startTime;
}
else
{
return time.TimeOfDay >= startTime &&
time.TimeOfDay <= endTime;
}
}Is this the way to go?
Solution
Here is another alternative...
Couple notes:
I think this is what ANeves meant by "would also consider extracting to a method the logic of whether or not to do the actions" but not sure so I decided to show it. By putting the entire decision tree in a method you have self documenting code (the "if (ShouldRunNow())" describes what you are trying to do without needing comments).
Eliminating "else" statements makes your code more readable (IMHO)
I personnally think using brackets when there is only one statement makes your code less readable as well. Of course, this is my own opinion and I even have people at my workplace that disagree with this. I will let the readers decide :)
Since the ShouldRunNow method is private then it doesn't hurt to not pass in the parameters. And putting them directly in the method will make them easier to find. This is not a big deal either way but just another alternative.
Update
Changed code based on feedback in comments
private void tmrMain_Tick(object sender, EventArgs e)
{
if (ShouldRunNow())
PerformClick();
}
private void PerformClick()
{
if (pnlAssembly.Visible)
cmdShed.PerformClick();
else if (pnlShed.Visible)
cmdServices.PerformClick();
else if (pnlWeb.Visible)
cmdAssembly.PerformClick();
}
private bool ShouldRunNow()
{
TimeSpan startTime = new TimeSpan(8, 0, 0);
TimeSpan endTime = new TimeSpan(18, 0, 0);
DateTime now = DateTime.Now;
// Only run Saturday and Sunday
if (now.DayOfWeek != DayOfWeek.Saturday && now.DayOfWeek != DayOfWeek.Sunday)
return false;
// Only run between the specified times.
if (endTime == startTime )
return true;
if (endTime = startTime;
return now.TimeOfDay >= startTime && now.TimeOfDay <= endTime;
}Couple notes:
I think this is what ANeves meant by "would also consider extracting to a method the logic of whether or not to do the actions" but not sure so I decided to show it. By putting the entire decision tree in a method you have self documenting code (the "if (ShouldRunNow())" describes what you are trying to do without needing comments).
Eliminating "else" statements makes your code more readable (IMHO)
I personnally think using brackets when there is only one statement makes your code less readable as well. Of course, this is my own opinion and I even have people at my workplace that disagree with this. I will let the readers decide :)
Since the ShouldRunNow method is private then it doesn't hurt to not pass in the parameters. And putting them directly in the method will make them easier to find. This is not a big deal either way but just another alternative.
Update
Changed code based on feedback in comments
Code Snippets
private void tmrMain_Tick(object sender, EventArgs e)
{
if (ShouldRunNow())
PerformClick();
}
private void PerformClick()
{
if (pnlAssembly.Visible)
cmdShed.PerformClick();
else if (pnlShed.Visible)
cmdServices.PerformClick();
else if (pnlWeb.Visible)
cmdAssembly.PerformClick();
}
private bool ShouldRunNow()
{
TimeSpan startTime = new TimeSpan(8, 0, 0);
TimeSpan endTime = new TimeSpan(18, 0, 0);
DateTime now = DateTime.Now;
// Only run Saturday and Sunday
if (now.DayOfWeek != DayOfWeek.Saturday && now.DayOfWeek != DayOfWeek.Sunday)
return false;
// Only run between the specified times.
if (endTime == startTime )
return true;
if (endTime < startTime)
return now.TimeOfDay <= endTime || now.TimeOfDay >= startTime;
return now.TimeOfDay >= startTime && now.TimeOfDay <= endTime;
}Context
StackExchange Code Review Q#17869, answer score: 6
Revisions (0)
No revisions yet.