6
\$\begingroup\$

An app has a feature that's much like any calendar application (like the Outlook calendar for example). Consequently, I need to do a lot of date logic. I wrote a class with date calculation methods that are useful in any scenario.

You can choose in what resolution of time you want to view the calendar, like a whole month, a week, a workweek, or a day.

Each calendar view corresponds to a date range. If you view the calendar by weeks, every time you browse to a given week or navigate to the previous/next week, a class generates a date range containing the start and end DateTime of the visible period in time.

For workweek resolution, this means the Saturday and Sunday are skipped and so the date ranges are not adjacent and don't "touch" each other.

The same principle applies to the month view. The start of the visible date range is not just the first day of the month. If the month's first day is not a Monday, a few days of the previous month are shown too, and are part of the visible date range. The same kind of logic applies for the end of the month.

My plan was to create classes that function as date range generators. For each type of calendar view a specific date range generator class knows how to create the requested period of time.

The interface:

/// <summary>
/// Represents a class that generates date ranges for loading appointmens
/// of an agenda.
/// </summary>
public interface IAgendaDateRangeGenerator
{
    /// <summary>
    /// Generates the date range that contains the given date.
    /// 
    /// Example case: when the class should generate date ranges of each week in the year,
    /// this method would generate a date range starting at monday and ending at sunday with 
    /// the given date within the date range. In other words, the generator creates the week of 
    /// the year that contains the given date.
    /// </summary>
    /// <param name="date"></param>
    /// <returns></returns>
    DateRange DateRangeForDay(DateTime date);

    /// <summary>
    /// Generates the previous period of time.
    /// </summary>
    /// <param name="dateRange"></param>
    /// <returns></returns>
    DateRange PreviousRange(DateRange dateRange);

    /// <summary>
    /// Generates the next period of time.
    /// </summary>
    /// <param name="dateRange"></param>
    /// <returns></returns>
    DateRange NextRange(DateRange dateRange);
}

The date helper class containing date calculation logic (mainly for context/part of review):

public class DateHelper
{
    public static DateTime EndOfDay(DateTime date)
    {
        return new DateTime(date.Year, date.Month, date.Day, 23, 59, 59, 999);
    }

    public static DateTime BeginOfDay(DateTime date)
    {
        return date.Date; // By returning date component, the time component will be zero values.
    }

    public static DateTime BeginOfWeekContaining(DateTime date)
    {
        int diff = (7 + (date.DayOfWeek - DayOfWeek.Monday)) % 7;
        return date.AddDays(-1 * diff);
    }

    public static DateTime EndOfWeekContaining(DateTime date)
    {
        DateTime monday = BeginOfWeekContaining(date);
        return monday.AddDays(6);
    }

    /// <summary>
    /// Gives the first date in future that's on the given DayOfWeek.
    /// Example, if the given date is a tuesday and the next following saturday is wanted
    /// this method returns the date of that saturday.
    /// Does not change time component.
    /// </summary>
    /// <param name="date"></param>
    /// <param name="dayOfWeek"></param>
    /// <returns></returns>
    public static DateTime FindFirstNextDay(DateTime date, DayOfWeek wantedDayOfWeek)
    {
        do
        {
            date = date.AddDays(1);
        }
        while (date.DayOfWeek != wantedDayOfWeek);
        return date;
    }

    /// <summary>
    /// Opposite of method FindFirstNextDay
    /// Looks in history until it finds the date occurence with the given DayOfWeek.
    /// Does not change time component.
    /// </summary>
    /// <param name="date"></param>
    /// <param name="wantedDayOfWeek"></param>
    /// <returns></returns>
    public static DateTime FindFirstPreviousDay(DateTime date, DayOfWeek wantedDayOfWeek)
    {
        do
        {
            date = date.AddDays(-1);
        }
        while (date.DayOfWeek != wantedDayOfWeek);
        return date;
    }

    /// <summary>
    /// Returns the date of the first day of the month that contains the given date.
    /// </summary>
    /// <param name="date"></param>
    /// <returns></returns>
    public static DateTime BeginOfMonthContaining(DateTime date)
    {
        int month = date.Month;
        while (date.AddDays(-1).Month == month)
        {
            date = date.AddDays(-1);
        }
        return date;
    }

    /// <summary>
    /// Returns date of last day of the month that contains the given date.
    /// </summary>
    /// <param name="date"></param>
    /// <returns></returns>
    public static DateTime EndOfMonthContaining(DateTime date)
    {
        int month = date.Month;
        while(date.AddDays(1).Month == month)
        {
            date = date.AddDays(1);
        }
        return date;
    }
}

Example of date range generator for workweeks (monday-friday):

public class WorkWeekDateRangeGenerator : IAgendaDateRangeGenerator
{
    public DateRange DateRangeForDay(DateTime date)
    {
        DateTime monday = DateHelper.BeginOfWeekContaining(date);
        DateTime friday = DateHelper.FindFirstNextDay(monday, DayOfWeek.Friday);
        return new DateRange(monday, friday);
    }

    public DateRange PreviousRange(DateRange fromRange)
    {
        DateTime start = DateHelper.FindFirstPreviousDay(fromRange.StartDate, DayOfWeek.Monday);
        DateTime end = DateHelper.FindFirstPreviousDay(fromRange.StartDate, DayOfWeek.Friday);
        return new DateRange(start, end);
    }

    public DateRange NextRange(DateRange fromRange)
    {
        DateTime start = DateHelper.FindFirstNextDay(fromRange.EndDate, DayOfWeek.Monday);
        DateTime end = DateHelper.FindFirstNextDay(fromRange.EndDate, DayOfWeek.Friday);
        return new DateRange(start, end);
    }
}

As you can see, the interface is designed to be stateless. This means the generator classes's PreviousRange and NextRange methods get a reference date range for determining the previous/next period. The class doesn't have to keep track of the current period. The outside world that uses the class stores the current date range and passes it to the PreviousRange or NextRange method of the generator class instance when navigating.

My biggest question is whether it is wise to have a stateless design. I ask thus mainly because the periods of time are not necessarily adjacent (like in my workweek generator example). If the design was not stateless, I'd probably store a week number in the generator class (in the case of the workweek example). Then I use the week number to create the date ranges and increase/decrease the week number each time a next/previous period is requested. It would be more like a state-machine then. I'm looking forward to your review and possible improvements or maybe even a totally different approach.

\$\endgroup\$

1 Answer 1

6
\$\begingroup\$

.NET 6 DateOnly

Much of what you are doing would be greatly simplified if you were using DateOnly that came out with .NET 6. So many nagging issues would fall by the wayside. For instance, with DateTime, one must be careful of the Kind property as well as the lingering time portion of the value.

Typically when working with calendars and using DateTime at midnight, many choose to make the starting DateTime inclusive and the ending DateTime exclusive, so that you can truly capture the full end date. With DateOnly, the ending date would be inclusive.

Much of my review will focus on the problems regarding using DateTime for such ranges.

Weekdays only, no Holidays

The code addresses a very specific purpose of your defined “work week”, which is simply week days Monday-Friday without regard to company holidays. If holidays were to be considered, the code becomes messier and typically would require inputting a list of known holiday DateTime values. Ideally with holidays the recommendation is a list of known holiday dates so that the code the library code is reusable for other uses and/or companies.

I have dabbled with a custom DateTimeRange class in the past, again with an InclusiveStartTime and ExclusiveEndTime, in which the times were not restricted to midnight. Because of this, my class would not be static. If you are restricting yourself to week days only, much of your code is simpler and maybe could possibly get by with being static.

UPDATE Why can it get away with static? Because your date ranges will never vary in length, or have different days of the week involved. It will always begin on start of day Monday. It will always be 5 days in duration. It will always end on end of day Friday. The previous week is always 7 days before the current Monday. The next week is always 7 days after the current Monday. Thus, you don’t need a class or struct that keeps track of a several properties. All you need is a singular DateTime instance, preferably with Kind of Unspecified, which is set to start of day on a Monday.

public static bool SundayIsFirstDayOfWeek { get; set; } = false;

public static DateTime GetCurrentMonday(DateTime date)
{
    int diff = DayOfWeek.Monday - date.DayOfWeek;
    if (date.DayOfWeek == DayOfWeek.Sunday && !SundayIsFirstDayOfWeek)
    {
        diff -= 7;
    }
    return DateTime.SpecifyKind(date.Date.AddDays(diff), DateTimeKind.Unspecified);
}

public static DateTime GetPreviousMonday(DateTime date) => GetCurrentMonday(date).AddDays(-7);
public static DateTime GetNextMonday(DateTime date) => GetCurrentMonday(date).AddDays(7);
public static DateTime GetEndOfCurrentWeek(DateTime date) => GetCurrentMonday(date).AddDays(5);
public static DateTime GetEndOfPreviousWeek(DateTime date) => GetPreviousMonday(date).AddDays(5);
public static DateTime GetEndOfNextWeek(DateTime date) => GetNextMonday(date).AddDays(5);

The above code brings together several of things I speak about through the remainder of my answer. I force the return Kind to be Unspecified, I ensure that the time component is set to midnight, and I have a property to declare if your week is defined as Monday-Sunday (SundayIsFirstDayOfWeek = false) or if it is Sunday-Saturday (SundayIsFirstDayOfWeek = true).

Specific Problems

Confusion over “Day”

Many of your methods use the phrase “Day” with DateTime objects. In .NET, “Day” has other meanings with other enums, including DateTime.Day property, which is a number ranging from 1 – 31, and not another DateTime instance as your code uses.

Start, End, and returned dates should use same Kind

Most likely, for week days only you would want your DateTime values to have a Kind property of Unspecified. Your code really gives no regard to Kind. Sometimes, the return value from a method returns the same kind as the input, and sometimes it does not.

Your original below returns the same kind:

public static DateTime BeginOfDay(DateTime date)
{
    return date.Date; // By returning date component, the time component will be zero values.
}

Though could be shorter as:

public static DateTime BeginOfDay(DateTime date) => date.Date; 

However, the below will always return Kind of Unspecified regardless of the input kind.

public static DateTime EndOfDay(DateTime date)
{
    return new DateTime(date.Year, date.Month, date.Day, 23, 59, 59, 999);
}

To ensure the same Kind,

public static DateTime EndOfDay(DateTime date) => new DateTime(date.Year, date.Month, date.Day, 23, 59, 59, 999, date.Kind);

OR

public static DateTime EndOfDay(DateTime date) => date.Date.AddDays(1).AddTicks(-1);

Where the returned DateTime is not only the same Kind, but also truly the inclusive end of the day the the tick rather than millisecond. I personally prefer working with exclusive end times, so my end of day’s midnight is same thing as start of next day’s midnight, in which case I would omit the AddTicks(-1).

Prefer direct calculation over brute force looping

My biggest complaint is with your looping in methods such as BeginOfMonthContaining or EndOfMonthContaining. Such looping is considered brute force. You can eliminate the loops for a more elegant solution, but what makes it elegant is that is just simple and direct.

public static DateTime BeginOfMonthContaining(DateTime date) =>  new DateTime(date.Year, date.Month, 1, 0, 0, 0, 0, date.Kind);

public static DateTime EndOfMonthContaining(DateTime date) => BeginOfMonthContaining(date).AddMonths(1).AddTicks(-1);

If using exclusive end times, then the AddTicks would be omitted and perhaps the method renamed, NextMonth.

An alternative to BeginOfMonthContaining could be:

public static DateTime BeginOfMonthContaining(DateTime date) => date.AddDays(date.Day – 1);

For FindFirstNextDay, the direct code could be:

public static DateTime FindFirstNextDay(DateTime date, DayOfWeek wantedDayOfWeek)
{
    int diff = wantedDayOfWeek - date.DayOfWeek;
    return date.Date.AddDays(diff < 1 ? diff + 7 : diff);
}

Note the return calls date.Date rather than date so that midnight start of day is returned. In other words, there is nothing preventing an input date to have a time other than midnight, but you want your method to return midnight.

Possible Confusion over BeginOfWeekContaining and EndOfWeekContaining

When you input a DateTime that falls on a Saturday or Sunday your BeginOfWeekContaining will always return the previous Monday, and that’s perfectly okay as implemented. One could argue that that is intentional behavior.

However, others could argue that since DayOfWeek.Sunday has integer code of 0, that is should return Monday of the current week, whereas Saturday falls in the previous week. I can see reasoning for either argument. But the method name by itself yields no clue, and there is no documentation clarifying the expected behavior. Perhaps a static property could be used on how Sunday is to be treated, i.e. does it fall on the end of the work week or the beginning of the work week.

Easier with DateOnly

From my code and concerns, there are a lot of potentional pitfalls using DateTime. It bears repeating that with .NET 6 and beyond, that DateOnly would eliminate many of these pitfalls and make the code easier and simpler to follow.

If you are stuck with .NET Framework, then NodaTime's LocalDate object can be used as an equivalent substitution for DateOnly. Pity to use NodaTime just for that little bit when what it is so much far capable when dealing with time zones and calendar systems.

\$\endgroup\$

Your Answer

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

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