5
\$\begingroup\$

This is my first attempt to model a day's time interval and the fact that the start and end are either inclusive or not:

public class UtcTimeOfDay
{
    public int HourOfDay { get; set; }
    public int MinuteOfDay { get; set; }
    public int SecondOfDay { get; set; }
}

public class UtcTimeInterval
{
    public UtcTimeOfDay StartUtcTimeOfDay { get; set; }
    public UtcTimeOfDay EndUtcTimeOfDay { get; set; }
    public bool StartInclusive { get; set; }
    public bool EndInclusive { get; set; }
}

Any suggestions for improvement?

\$\endgroup\$

3 Answers 3

5
\$\begingroup\$

Do you need UtcTimeOfDay at all? DateTime already has useful methods for working with paramterized times.

The disadvantage that using a raw DateTime has is that you have no compile-time guarantee that you won't get an invalid DateTime (e.g. one with a Day component above 0). However, you can get the best of both worlds by having your class wrap a DateTime:

public class UtcTimeOfDay
{
    public readonly DateTime Value;

    public UtcTimeOfDay(int hour, int minute, int second)
    {
        Value = new DateTime(0,0,0,hour,minute,second);
    }
}

Now you get all the advantages of the rich functionality of DateTime, along with the guarantees provided by your original class.


Regarding your original UtcTimeOfDay, the names aren't great. Shouldn't MinuteOfDay be MinuteOfHour, and likewise shouldn't SecondOfDay be SecondOfMinute?

This also reveals another problem: there's no guarantee that these values are valid (they could be negative, or above the maximum possible). This could be done with validation in a constructor, making the setters private.


The name UtcTimeInterval doesn't make any reference to the fact that it's specifically for a time interval within a day. Why not UtcTimeOfDayInterval?


While you can use StartInclusive and EndInclusive, a normal technique to deal with exclusive time spans is simply to add or subtract a tick to your time. So for example, if the inclusive start time was startTimeInclusive, then if you want an exclusive version, that would be startTimeExclusive = startTimeInclusive.AddTicks(1);

Being able to do this so simply is another benefit of using a DateTime rather than trying to roll your own structure to represent the same thing.

So what I would do is remove the StartInclusive and EndInclusive properties, and do one of the following:

  • Just let whoever sets the start and end UtcTimeOfDay take responsibility for adding or subtracting a tick if they need to.
  • Add a constructor with boolean startInclusive and endInclusive parameters, as well as the start and end UtcTimeOfDays which itself does the tick adding/subtracting when needed.

Update

As a couple of people have noted in comments, using a TimeSpan instead of a DateTime may be preferable, as it is more in fitting with the meaning of these structures. This makes the add/subtract tick logic slightly more verbose, but not prohibitively.

\$\endgroup\$
7
  • \$\begingroup\$ The most common way of indicating a time of day in .NET is a TimeSpan representing the time since midnight. \$\endgroup\$
    – Jon Hanna
    Apr 21, 2015 at 17:38
  • \$\begingroup\$ @JonHanna Interesting. I can see why that might be more natural than what I described. \$\endgroup\$ Apr 21, 2015 at 17:39
  • \$\begingroup\$ Indeed, the code you have here will throw ArgumentOutOfRangeException while conversly, DateTime.TimeOfDay is an example where TimeSpan does this rôle in the BCL. \$\endgroup\$
    – Jon Hanna
    Apr 21, 2015 at 18:39
  • \$\begingroup\$ Time since midnight fails with daylight savings \$\endgroup\$
    – Ewan
    Apr 30, 2015 at 16:31
  • \$\begingroup\$ @Ewan Can you give an example of that? \$\endgroup\$ Apr 30, 2015 at 16:52
4
\$\begingroup\$

In general:

  • Put them in a namespace so they don't pollute the global namespace.
  • Are you aware of the .NET struct DateTime and struct TimeSpan ? They do almost exactly what you're looking for. Why don't you make use of them?
  • Part of the reason to use classes is so that you can define them to behave consistently. UtcTimeInterval is a poor abstraction for what you're trying to do, for a number of reasons:
    • It doesn't enforce a canonical representation. There are multiple possible representations of a time, say 2am: HourOfDay=1; MinuteOfDay=60 and HourOfDay=2; MinuteOfDay=0; It's even possible for a time that is actually later to be compared as lexicographically earlier. Also are negatives supposed to be possible
    • The data structure is supposed to only allow itself to be set to sensible values. If nonsense is put in, it should throw an exception or mark itself invalid.
    • And, have you considered leap seconds?
    • Why are you using "1 second" as the smallest nonzero interval? Many applications need finer-grained than that.
  • Neither class supports any operations except the defaults for the class: assignment, equality, comparison. Why is there no method that takes two UtcTimeOfDays and returns a UtcTimeInterval? What are these classes going to be used for?
\$\endgroup\$
1
  • \$\begingroup\$ Ok makes all sense. There are certain reason for my choice but your point reg. canonical representation I take def. on board. \$\endgroup\$
    – cs0815
    Apr 21, 2015 at 13:58
1
\$\begingroup\$

I've also had the pleasure of working with periods of time which start at a particular time of the day and I ended up with a similar Hour/min/sec of day structure to yours, although it confuses me that you call it UTCTimeOfDay?

The problems with modeling things like shifts which start at 7:30am with the standard date time objects are

1: Timezones! this is why the UTC part of your struct confuses me. normally you would want the shift (or whatever) to start at 7:30am local time where ever in the world the shop is.

2: Daylight savings, This stops you using TimeSpan, 7 and a half hours since midnight is not always 7:30am

Storing int hour,min and second is really your only good solution

But then you have the problem of calculating the difference between two of these 'time of days'. This gives you the same problems but in reverse. you have to convert '7:30am' to '7:30am on this particular day in this particular timezone' then again with the end TimeOfDay. once thats done you can use the standard datetime - datetime = timespan to work out the actual elapsed time

Again its non intuitive when the shift covers daylight saving changes, leap years etc

Start and End Inclusive/Exclusiveness is only really a problem when you are looking for overlaps. I always treat the end as exclusive and the start as inclusive. This allows midnight to midnight to be one day rather than having 12:59:59.9999 mucking up your maths

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.