A switch case statement that returns a timespan

Question

I have a switch case statement containing enum values as cases. These enum values are time units. Now I want to generate a timespan by using an enum value and a long value so like this:

2 (long) Minutes (Enum value)

1 (long) Year (Enum value)

... and so on.

This is the switch-case statement:

private const double yearValue = 365.2425;

public TimeSpan ResetTimeSpan
            {
                get
                {
                    switch (ResetTimeUnitEnumValue)
                    {
                        case TimeUnit.Seconds:
                            return new TimeSpan(0,0,0, (int)ResetTime);
                        case TimeUnit.Minutes:
                            return new TimeSpan(0, 0, (int)ResetTime, 0);
                        case TimeUnit.Hours:
                            return new TimeSpan(0, (int)ResetTime, 0);
                        case TimeUnit.Days:
                            return new TimeSpan((int)ResetTime, 0, 0, 0);
                        case TimeUnit.Months:
                            return new TimeSpan((int)(ResetTime * (yearValue / 12)), 0, 0, 0);
                        case TimeUnit.Years:
                            return new TimeSpan((int)(ResetTime * yearValue), 0, 0, 0);
                        default:
                            throw new ArgumentOutOfRangeException();
                    }
                }
            }

But I don't like it and it feels very double. Could anyone review this and refactor it, since I'm not able to see it.

Answer 1

private const double yearMultiplier = 365.2425;

private Dictionary<TimeUnit, Func<double, TimeSpan>> timespanConverters = new Dictionary<TimeUnit, Func<double, TimeSpan>>
{
    { TimeUnit.Seconds, TimeSpan.FromSeconds },
    { TimeUnit.Minutes, TimeSpan.FromMinutes },
    { TimeUnit.Hours,   TimeSpan.FromHours },
    { TimeUnit.Days,    TimeSpan.FromDays },
    { TimeUnit.Months, t => TimeSpan.FromDays((int)t * (yearMultiplier / 12))) },
    { TimeUnit.Years,  t => TimeSpan.FromDays((int)t * yearMultiplier)) }
}

// ....

public TimeSpan ResetTimeSpan
{
    get
    {
        return timespanConverters[ResetTimeUnitEnumValue](ResetTime);
    }
}

Bit of a shame that System.TimeSpan doesn't have FromMonths or FromYears methods otherwise this would look really neat :)

Answer 2

I don't think you're going to get too much cleaner, but I'll offer up a few alternatives and thoughts.

First, you could use the TimeSpan convenience methods (TimeSpan.FromSeconds, FromMinutes, FromHours, and FromDays) to at least get up to days. FWIW, I think that's more readable than using the constructor parameters. Alternatively, using named arguments to the constructor would help readability.

You could abstract out the calculation to a map:

private const double yearValue = 365.2425;

static readonly timeUnitInSeconds = new Dictonary<TimeUnit, double>() {
   { TimeUnit.Seconds, 1 },
   { TimeUnit.Minutes, 60 },
   { TimeUnit.Hours, 60 * 60 },
   { TimeUnit.Days, 60 * 60 * 24 },
   { TimeUnit.Months, 60 * 60 * 24 * (yearValue / 12) }, 
   { TimeUnit.Years, 60 * 60 * 24 * yearValue }
};

get { 
   return TimeSpan.FromSeconds(timeUnitInSeconds[ResetTimeUnitEnumValue] * (int)ResetTime);
}

which is a little cleaner, and makes the calcs easier to read.

Or, you could go for the whole shebang, and make the TimeUnit enum itself the value in seconds:

private const double yearValue = 365.2425;

enum TimeUnit {
   // in seconds
   Seconds = 1,
   Minutes = 60,
   Hours = 60 * 60,
   Days = 60 * 60 * 24,
   Months = (int)(60 * 60 * 24 * (yearValue / 12))),
   Years = (int)(60 * 60 * 24 * yearValue)
}

get {
   return TimeSpan.FromSeconds((int)ResetTimeUnitEnumValue * (int)ResetTime);
}

That's probably bit too clever by half, though. Also note the int conversion is done first there, which could lead to slightly different results if you don't have a whole # of seconds. Your month value is a bit odd (it should be closer to 30.5 if it's intended to be the average month length in days) so you may want to double check that one.