CodeSOD: Eventful Timing

I once built a system with the job of tracking various laboratory instruments, and sending out notifications when they needed to be calibrated. The rules for when different instruments triggered notifications, and when notifications should be sent, and so on, were very complicated.

An Anonymous reader has a similar problem. They're tracking "Events"- like seminars and conferences. These multi-day events often have an end date, but some of them are actually open ended events. They need to, given an event, be able to tell you how much it costs. And our Anonymous reader's co-worker came up with this solution to that problem:

public class RunningEventViewModel : SingleDataViewModel<EventData>{ private DateTime _now; private readonly Timer _timer; public RunningEventViewModel(EventData data) : base(data) { _now = DateTime.Now; _timer = new Timer(x => { _now = DateTime.Now; NotifyOfPropertyChange(() => DurationDays); NotifyOfPropertyChange(() => DurationHours); NotifyOfPropertyChange(() => DurationMinutes); NotifyOfPropertyChange(() => DurationSeconds); NotifyOfPropertyChange(() => DurationString); NotifyOfPropertyChange(() => TotalCost); }); if (!Stop.HasValue) { _timer.Change(900, 900); } } public int DurationSeconds { get { return ((Stop ?? _now) - Start).Seconds; } } public decimal TotalCost { get { var end = Stop ?? _now; const int SecsPerDay = 3600 * 24; decimal days = ((int)(end - Start).Duration().TotalSeconds) / (decimal)SecsPerDay; return (DailyCosts.HasValue ? DailyCosts.Value : 0) * days; } } public int DurationDays { get { return ((Stop ?? _now) - Start).Days; } } public int DurationHours { get { return ((Stop ?? _now) - Start).Hours; } } public int DurationMinutes { get { return ((Stop ?? _now) - Start).Minutes; } } public TimeSpan Duration { get { return (Stop ?? _now) - Start; } } public string FromTo { get { var res = String.Empty; if (Data.EventStart.HasValue) { res += Data.EventStart.Value.ToShortDateString(); if (Data.EventStop.HasValue) { res += " - " + Data.EventStop.Value.ToShortDateString(); } } return res; } } public string DurationString { get { var duration = ((Stop ?? _now) - Start).Duration(); var res = new StringBuilder(); if (DurationDays > 0) { res.Append(duration.Days).Append(" days "); } if (DurationHours > 0) { res.Append(duration.Hours).Append(" hours "); } if (DurationMinutes > 0) { res.Append(duration.Minutes).Append(" minutes"); } return res.ToString().TrimEnd(' '); } } public DateTime Start { get { return Get(Data.EventStart.Value, x => x.ToLocalTime().ToUniversalTime()); } } public DateTime? Stop { get { return (DateTime?)null; } } public decimal? DailyCosts { get { return Data.DailyCost; } } private static TResult Get<TItem, TResult>(TItem item, Func<TItem, TResult> resultSelector) { // this method is actually an extension method from our core framework, it's added here for better understanding the code if(Equals(item, null)) { return default(TResult); } return resultSelector(item); }}

Is null a mistake? Well, it certainly makes this code more complicated. Speaking of more complicated, I like how this class is responsible for tracking a Timer object so that it can periodically emit events. So much for single-responsibility, and good luck during unit testing.

The real WTF here, however, is that .NET has a rich and developer-friendly date-time API, which already has pre-built date difference functions. Code like this block in TotalCost:

 const int SecsPerDay = 3600 * 24; decimal days = ((int)(end - Start).Duration().TotalSeconds) / (decimal)SecsPerDay;

" is completely unnecessary. Similarly, the FromTo and DurationString functions could benefit from a little use of the TimeSpan object, and a little String.format. And it's clear that the developer knows these exist, because they've used them, yet in TotalCost, it's apparently too much to bear.

But the real topper, on this, is the Stop property. Once you look at that, most of the code in this file becomes downright stupid.

[Advertisement] Release!is a light card game about software and the people who make it. Play with 2-5 people, or up to 10 with two copies - only $9.95 shipped!