Task scheduler for a particular application

Question

This isn't meant as a scheduler for just any job. The job that I need to schedule here is very specific for an application I'm writing for one of my clients.

The scheduler will run as a console application because it's comparatively lightweight. The application will be started with Windows (startup items in the Registry) and should always be running - much like a Windows service but not (mostly because I don't have a project template for a Windows service in VS 2013 Community).

The task this scheduler runs should be done at 4am every day.

Does it look like the code below matches this description of the requirements?

class MyScheduledTask
{
    private static Timer _timer;

    static void Main(string[] args)
    {
        SetTime();
    }

    private static void SetTime()
    {
        DateTime currentTime = DateTime.Now;
        int interval = 0;
        DateTime scheduleTime = new DateTime(currentTime.Year, currentTime.Month, currentTime.Day, 12, 45, 0);

        if (currentTime <= scheduleTime)
        {
            interval = (int)scheduleTime.Subtract(currentTime).TotalSeconds;
        }
        else
        {
            interval = (int)scheduleTime.AddDays(1).Subtract(currentTime).TotalSeconds;
        }

        _timer = new Timer(interval);
        _timer.Elapsed += new ElapsedEventHandler(_timer_Elapsed);
        _timer.Start();
    }

    void _timer_Elapsed(object sender, System.Timers.ElapsedEventArgs e)
    {
        StartTaskWork();
        _timer.Interval = (24 * 60 * 60 * 1000);
    }
}

Answer 1

First, I'd mention that scheduling execution of anything in Windows is better done through the Windows Task Scheduler. Scheduling your application that way would remove the "must run everyday at 4AM" requirement from the code, although I don't see how your code is enforcing that.

That said...

Magic Numbers & Configuration

DateTime scheduleTime = new DateTime(currentTime.Year, currentTime.Month, currentTime.Day, 12, 45, 0);

"12" and "45" (and "0") are magic numbers here. These should be taken from configuration, or better yet, from command-line arguments.

---

The right hammer for the right nail

System.Int32 is an awesome hammer. But that doesn't make every number a nail.

int interval = 0;

You (present you - future you could be a different story) know they're seconds. But the name says nothing of it, and the type is making your life harder than it needs to be. The .NET framework has a struct specifically meant to represent a time interval, more precisely a time span - System.TimeSpan. Compare readability here:

TimeSpan interval = TimeSpan.FromSeconds(0);

Ambiguity: gone. And as a bonus, you no longer need to cast anything here:

    if (currentTime <= scheduleTime)
    {
        interval = scheduleTime.Subtract(currentTime);
    }
    else
    {
        interval = scheduleTime.AddDays(1).Subtract(currentTime);
    }

Oh, and it turns this:

_timer.Interval = (24 * 60 * 60 * 1000);

Into that:

_timer.Interval = TimeSpan.FromDays(1).TotalMilliseconds;

And that rids the code of an implicit conversion from int to double, since both TimeSpan.TotalMilliseconds and Timer.Interval are double.

Answer 2

The code does not match the description of the requirements. It calculates the time based on 12:45 PM, instead of 4:00 AM.

One problem with your posting is that it's not immediately clear which Timer you are using. (For edification of anyone else reading this, the most likely are System.Timers.Timer or System.Threading.Timer; the other possible Timers are System.Windows.Forms.Timer and System.Web.UI.Timer but they wouldn't be used in a console app.) The problem is, the code you posted is not complete because it doesn't contain the using statements.

On second reading I saw that _timer_Elapsed() takes a System.Timers.ElapsedEventArgs so that settles it, but it shouldn't have been necessary for me to pore over your code to determine the type you're using.

I think your code would be simpler if you used a System.Threading.Timer instead. You don't need the thread-safety that System.Timers.Timer provides, and you would benefit greatly from the ability to schedule the first timer event at a different interval than the rest of the timer events -- you wouldn't have to change _timer.Interval inside _timer_Elapsed(). So it doesn't matter that it doesn't have the standard timer object model.

Here's my take:

using Timer=System.Threading.Timer;
class MyScheduledTask
{
    private static Timer dailyWakeupCall;

    static void Main(string[] args)
    {
        SetTime();
        Thread.Sleep(System.Threading.Timeout.Infinite);
    }

    private static void SetTime()
    {
        DateTime currentTime = DateTime.Now;
        DateTime scheduleTime = new DateTime(currentTime.Year, currentTime.Month, currentTime.Day, 12, 45, 0);

        TimeSpan period = new TimeSpan(1,0,0,0); // days, hours, minutes, seconds
        // or you could use @Mat's suggestion and say = TimeSpan.FromDays(1.0);
        TimeSpan initialInterval;
        if (currentTime <= scheduleTime)
        {
            initialInterval = scheduleTime.Subtract(currentTime);
        }
        else
        {
            initialInterval = scheduleTime.AddDays(1).Subtract(currentTime);
        }

        dailyWakeupCall = new Timer(_timerElapsed, null, initialInterval, period);
    }

    void _timer_Elapsed(object state)
    {
        StartTaskWork();
    }
}