- 6.7k
- 1
- 20
- 32
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).
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.
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.
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).
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.
This post went a full year without any answers. I imagine this is far too late to help the OP but for academic purposes, I will post an answer.
This post went a full year without any answers. I imagine this is far too late to help the OP but for academic purposes, I will post an answer.
This post went a full year without any answers. I imagine this is far too late to help the OP but for academic purposes, I will post an answer.
.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.
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.