9
\$\begingroup\$

Just wondering if anyone had any suggestions for improvements. I created these extension methods so it's easier to get the start and end dates of Daylight Savings Time from the TimeZoneInfo class.

public static class ExtensionMethods
{
    /// <summary>
    /// Gets the TimeZoneInfo.AdjustmentRule in effect for the given year.
    /// </summary>
    /// <param name="timeZoneInfo"></param>
    /// <param name="year"></param>
    /// <returns></returns>
    public static TimeZoneInfo.AdjustmentRule GetAdjustmentRuleForYear(this TimeZoneInfo timeZoneInfo, int year)
    {
        TimeZoneInfo.AdjustmentRule[] adjustments = timeZoneInfo.GetAdjustmentRules();
        // Iterate adjustment rules for time zone 
        foreach (TimeZoneInfo.AdjustmentRule adjustment in adjustments)
        {
            // Determine if this adjustment rule covers year desired 
            if (adjustment.DateStart.Year <= year && adjustment.DateEnd.Year >= year)
            {
                return adjustment;
            }
        }
        return null;
    }

    /// <summary>
    /// Gets the Daylight Savings Time start date for the given year.
    /// </summary>
    /// <param name="adjustmentRule"></param>
    /// <param name="year"></param>
    /// <returns></returns>
    public static DateTime GetDaylightTransitionStartForYear(this TimeZoneInfo.AdjustmentRule adjustmentRule, int year)
    {
        return adjustmentRule.DaylightTransitionStart.GetDateForYear(year);
    }

    /// <summary>
    /// Gets the Daylight Savings Time end date for the given year.
    /// </summary>
    /// <param name="adjustmentRule"></param>
    /// <param name="year"></param>
    /// <returns></returns>
    public static DateTime GetDaylightTransitionEndForYear(this TimeZoneInfo.AdjustmentRule adjustmentRule, int year)
    {
        return adjustmentRule.DaylightTransitionEnd.GetDateForYear(year);
    }

    /// <summary>
    /// Gets the date of the transition for the given year.
    /// </summary>
    /// <param name="transitionTime"></param>
    /// <param name="year"></param>
    /// <returns></returns>
    public static DateTime GetDateForYear(this TimeZoneInfo.TransitionTime transitionTime, int year)
    {
        if (transitionTime.IsFixedDateRule)
        {
            return GetFixedDateRuleDate(transitionTime, year);
        }
        else
        {
            return GetFloatingDateRuleDate(transitionTime, year);
        }
    }

    private static DateTime GetFixedDateRuleDate(TimeZoneInfo.TransitionTime transitionTime, int year)
    {
        return new DateTime(year,
                           transitionTime.Month,
                           transitionTime.Day,
                           transitionTime.TimeOfDay.Hour,
                           transitionTime.TimeOfDay.Minute,
                           transitionTime.TimeOfDay.Second,
                           DateTimeKind.Unspecified);
    }

    private static DateTime GetFloatingDateRuleDate(TimeZoneInfo.TransitionTime transitionTime, int year)
    {
        // For non-fixed date rules, get local calendar
        Calendar localCalendar = CultureInfo.CurrentCulture.Calendar;

        // Get first day of week for transition
        // For example, the 3rd week starts no earlier than the 15th of the month
        int startOfWeek = transitionTime.Week * 7 - 6;

        // What day of the week does the month start on?
        int firstDayOfWeek = (int)localCalendar.GetDayOfWeek(new DateTime(year, transitionTime.Month, 1));

        // Determine how much start date has to be adjusted
        int transitionDay;
        int changeDayOfWeek = (int)transitionTime.DayOfWeek;
        if (firstDayOfWeek <= changeDayOfWeek)
            transitionDay = startOfWeek + (changeDayOfWeek - firstDayOfWeek);
        else
            transitionDay = startOfWeek + (7 - firstDayOfWeek + changeDayOfWeek);

        // Adjust for months with no fifth week
        if (transitionDay > localCalendar.GetDaysInMonth(year, transitionTime.Month))
            transitionDay -= 7;

        return new DateTime(year, 
                   transitionTime.Month, 
                   transitionDay,
                   transitionTime.TimeOfDay.Hour,
                   transitionTime.TimeOfDay.Minute,
                   transitionTime.TimeOfDay.Second,
                   DateTimeKind.Unspecified);
    }
}
\$\endgroup\$
3
  • 2
    \$\begingroup\$ Just a quick comment: ExtensionMethods is a rather vague name for a class. Remember that these methods can also be called as static methods - hence, I'd suggest you rename it to TimeZones, so you can call TimeZones.GetAdjustmentRuleForYear(foo, 2014);... or perhaps TimeZoneExtensions would be better. In any case, ExtensionMethods is too vague for such a specialized class. \$\endgroup\$ Sep 17, 2014 at 18:07
  • 4
    \$\begingroup\$ No offense, but this is not a situation where you should roll your own. This is why. \$\endgroup\$
    – RubberDuck
    Sep 17, 2014 at 18:20
  • \$\begingroup\$ That's a good suggestion about the class name. It was just something I threw together for testing but I'm guilty of not using a specific name in other cases. And unfortunately, I don't have a choice about working with time zones. \$\endgroup\$
    – M. Cooper
    Sep 17, 2014 at 18:43

1 Answer 1

4
\$\begingroup\$

Room for improvement:

  • ExtensionMethods is a terrible name. What are we extending? We're extending the TimeZoneInfo class, so we should probably call our extension methods something like TimeZoneExtensions. There are lots of options for naming schemes. Pick one you like.

  • GetAdjustmentRuleForYear can return null. I'm not a fan of that. What's worse is the XML doc doesn't even mention it. You should at least document it, but I would personally at least consider throwing an exception.

  • Speaking of null, it's possible for this to be null, so you should also consider checking for it.

  • This is a pretty useless comment. It serves no purpose but to clutter the code.

     // Iterate adjustment rules for time zone 
    foreach (TimeZoneInfo.AdjustmentRule adjustment in adjustments)
    
  • firstDayOfWeek is a misleading name. It looks like it's really the first day of the month. I suggest firstWeedayOfMonth instead.

     // What day of the week does the month start on?
    int firstDayOfWeek = (int)localCalendar.GetDayOfWeek(new DateTime(year, transitionTime.Month, 1));
    
  • If you need a comment to say what, you generally need to extract a well named method instead.

    // Determine how much start date has to be adjusted
    int transitionDay;
    int changeDayOfWeek = (int)transitionTime.DayOfWeek;
    if (firstDayOfWeek <= changeDayOfWeek)
        transitionDay = startOfWeek + (changeDayOfWeek - firstDayOfWeek);
    else
        transitionDay = startOfWeek + (7 - firstDayOfWeek + changeDayOfWeek);
    
  • That statement could also use some braces in my opinion.

What I like:

  • Use of XML docs on public methods.
  • You've generally used pretty good names (other than ones I've mentioned).
  • It's fairly readable code.
  • Methods are short and to the point with the exception of GetFixedDateRuleDate.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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