Simple ConsoleWorker

Question

This class updates the console's title so it can tell the person using it the programs uptime, which can be pretty useful. The class holds a timer and each time the timer executes it will call the function to update the title (interval is in config file).

How can I improve on this?

using Apple.Application.Base.Config;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using System.Timers;

namespace Apple.Application.Base.Core
{
    class ConsoleWorker
    {
        /// <summary>
        /// Console timer
        /// </summary>
        private Timer _consoleTimer;

        /// <summary>
        /// ConsoleWorker
        /// </summary>
        /// <param name="config">Instance of AppleConfig</param>
        public ConsoleWorker(AppleConfig config)
        {
            _consoleTimer = new Timer(int.Parse(config.GetConfigElement("console.timer.interval")));
            _consoleTimer.Elapsed += new ElapsedEventHandler(OnElapsed);
            _consoleTimer.Enabled = true; // Enable it
        }

        public void OnElapsed(object sender, ElapsedEventArgs e)
        {
            DateTime Startup = Apple.ServerInformation.ServerStarted;
            TimeSpan Uptime = DateTime.Now - Startup;

            string days = Uptime.Days + " day" + (Uptime.Days != 1 ? "s" : "") + ", ";
            string hours = Uptime.Hours + " hour" + (Uptime.Hours != 1 ? "s" : "") + ", and ";
            string mins = Uptime.Minutes + " min" + (Uptime.Minutes != 1 ? "s" : "");
            string UptimeString = days + hours + mins;

            Console.Title = "Apple Server [" + UptimeString + "]";
        }
    }
}

Answer 1

I think for the given task, the code is fine.

If you want to do some architecting, here are some thoughts

• I'd say the class does two things, timing and updating console
• I'd create a worker class that would do the timer stuff
• Inject (using dependency injection) an interface that can output stuff
• Create an interface that defines "outputting stuff"
• Create a console version of that interface
• Feed it to the concrete one

It's just refactoring so that you can use the same stuff to write output to different places such as disk, web service etc..

Answer 2

I love that you're storing the interval in a config, but there's some issues with your implementation of that.

1. This class doesn't need the whole config file.

   There's no reason for this class to have knowledge of the entire configuration and how to access it. It only needs to know what the interval should be. I'd change your constructor so that you're just passing the interval.

2. GetConfigSection isn't a very nice way to access the configuration.

   It relies on a string key and string keys are very easy to misspell. It's much preferable to access these through the strong typing option that Settings.Default provides. See here for more info.

---

Lastly, this block looks like a wonderful place to extract a method and use String.Format

string days = Uptime.Days + " day" + (Uptime.Days != 1 ? "s" : "") + ", ";

string hours = Uptime.Hours + " hour" + (Uptime.Hours != 1 ? "s" : "") + ", and ";

string mins = Uptime.Minutes + " min" + (Uptime.Minutes != 1 ? "s" : "");

string UptimeString = days + hours + mins;

For example:

string days = String.Format("{0} day{1}  , ", Uptime.Days, (Uptime.Days != 1 ? "s" : ""));

Or, even better, but only if you're using C# 6

string days = $"{Uptime.Days} day{(Uptime.Days != 1 ? "s" : "")}  , ";

Once you do this for all 3, you'll see pretty quickly your opportunity to extract a method.