Simple timer checking class

Question

This is a simple class that runs a timer every 10 seconds to update the title with some information like how long the application has been up, depending on the datetime that is set in another class when the application has finished booting up and is responsive. This application is part of a game server and it just displays how long the server has been up so I know when it was last launched.

In the future it will also include the number of players connected in the title. it is important to continuously reload it as this is a console application and there isn't really much room anywhere else to display such data.

I'm really not sure about the method name CheckServer but I was feeling very unmotivated at the time, if anyone could think of a better name, be sure to post that too.

I'm looking for basic improvements in overall performance.

internal sealed class ServerProcessWorker : IDisposable
{
    private Timer _serverTimer;
    private readonly int _timerInterval;
    private DateTime _lastUpdate;
    private bool _disposing;

    public ServerProcessWorker()
    {
        _timerInterval = 10000;
    }

    public void Start()
    {
        _serverTimer = new System.Threading.Timer(e => CheckServer(), null, TimeSpan.Zero, TimeSpan.FromMilliseconds(_timerInterval));
    }

    private void CheckServer()
    {
        if ((DateTime.Now - _lastUpdate).TotalMilliseconds <= _timerInterval )
        {
            return;
        }

        var serverUptime = DateTime.Now - Sirius.GetServer().GetServerInformation().ServerStarted;

        var days = serverUptime.Days + " day" + (serverUptime.Days != 1 ? "s" : "") + ", ";
        var hours = serverUptime.Hours + " hour" + (serverUptime.Hours != 1 ? "s" : "") + ", and ";
        var minutes = serverUptime.Minutes + " minute" + (serverUptime.Minutes != 1 ? "s" : "");
        var uptimeString = days + hours + minutes;

        Console.Title = Sirius.GetServer().GetServerInformation().ServerName + " - Uptime: " + uptimeString;

        _lastUpdate = DateTime.Now;
    }

    public void Dispose()
    {
        if (_disposing)
        {
            return;
        }

        _disposing = true;

        _serverTimer.Dispose();
        _serverTimer = null;
    }
}

Answer 1

This check

if ((DateTime.Now - _lastUpdate).TotalMilliseconds <= _timerInterval )
{
    return;
}  

is superflous, because the timer fires at a defined interval.

---

Does the ServerName change over the time the application is running ? I guess not so it would be better to request the name once and store it for later usage.

The same applies to ServerStarted.

---

Personally I would create an extension method for the TimeSpan which would do the whole work with the day(s), hour(s) and minute(s). Something along these lines

public static string ToWords(this TimeSpan timeSpan)
{
    return  timeSpan.Days + (timeSpan.Days != 1 ? " days" : " day") + ", "
          + timeSpan.Hours  + (timeSpan.Hours != 1 ? " hours" : " hour") + ", and "
          + timeSpan.Minutes  + (timeSpan.Minutes != 1 ? " minutes" : " minute");
}

and calling like so

var uptimeString = serverUptime.ToWords();  

Disclaimer: I don't like the name of the method.

Answer 2

Three bits of feedback for you.

DateTimeOffset

Your timer will show funny information during the hour transition to/from DST. Consider using a DateTimeOffset instead.

Cache that reference

To improve performance, maybe you should cache something here:

var serverUptime = DateTime.Now - Sirius.GetServer().GetServerInformation().ServerStarted;
//--Snip--
Console.Title = Sirius.GetServer().GetServerInformation().ServerName + " -Uptime: " + uptimeString;

For example

var serverInformation = Sirius.GetServer().GetServerInformation();
var serverUptime = DateTime.Now - serverInformation.ServerStarted;
//--Snip--
Console.Title = serverInformation.ServerName + " -Uptime: " + uptimeString;

Catch exceptions

If you have an unhandled exception in a worker thread, it is very very bad. Since CheckServer is not exactly critical, I would wrap the entire thing in a try/catch block and just swallow any exceptions.