HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Using a recurring System.Threading.Timer in an Mvc Application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
mvcapplicationsystemrecurringthreadingtimerusing

Problem

I'd like your opinion on something. I have the following class:

public class ApplicationClock
{
    void tick(object _)
    {
        lock (tickLock)
        {
            try
            {
                MessageBroker.Publisher.Publish(new Tick());
            }
            catch (Exception e)
            {
                this.Logger().Error("Application clock tick error", e);
                //who knows why this happened, let's try and restart
                if (ticker != null) ticker.Dispose();
                Start();
            }
        }
    }
    readonly Object tickLock = new Object();
    Timer ticker = null;
    public ApplicationClock Start()
    {
        ticker = new Timer(tick, null, 0, 60*1000);
        return this;
    }
}


launched from the MvcApplication bootstrapper

...
ApplicationClock clock;
protected void Application_Start() 
{
   //...
   clock = new ApplicationClock().Start();
}


I realize that a service and some sort of inter-process communication (whether windows inter-process communication, HTTP, or something else) is more standard and reliable but I have a frequently changing team and don't want to add another step that is necessary to run the app.

This seems to work but I've only launched it in a dev scenario. Am I missing anything that would cause problems in production?

That's a System.Threading.Timer by the way.

Solution

Looks mostly ok, just some issues around coupling:

Consider one of the following

  • Pass MessageBroker.Publisher via the ApplicationClock constructor as an external dependency (preferably an interface). This will make the current implicit dependency explicit and visible which should yield in better maintenance in the future and easier unit testing.



  • Pass the tick handler in as an Action parameter - no dependency on a specific object at all.



  • Expose a ClockTick event and raise the event handler on tick. This way anyone interested in the clock can subscribe and the clock doesn't need to know about them at all.



Also your tick interval is hard coded, it should be passed in as parameter.

Context

StackExchange Code Review Q#15414, answer score: 5

Revisions (0)

No revisions yet.