Get remaining days in date range

Question

I wonder if there are some possible ways to simplify my code.

startDate = '04/02/2014'
endDate = '04/06/2014'
mondayTag = 0
tuesdayTag = 0
wednesdayTag = 1
thursdayTag = 0
fridayTag = 1
saturdayTag = 0
sundayTag = 0
public int GetRemainingDays(DateTime startDate, DateTime endDate, bool mondayTag, bool tuesdayTag, bool wednesdayTag, bool thursdayTag, bool fridayTag, bool saturdayTag, bool sundayTag)
{
 int i = 0;
 for (DateTime day = startDate.AddDays(1); day.Date <= endDate.Date; day = day.AddDays(1))
 {
 if (day.DayOfWeek == DayOfWeek.Monday && mondayTag == true)
 {
 i = i + 1;
 }
 else if (day.DayOfWeek == DayOfWeek.Tuesday && tuesdayTag == true)
 {
 i = i + 1;
 }
 else if (day.DayOfWeek == DayOfWeek.Wednesday && wednesdayTag == true)
 {
 i = i + 1;
 }
 else if (day.DayOfWeek == DayOfWeek.Thursday && thursdayTag == true)
 {
 i = i + 1;
 }
 else if (day.DayOfWeek == DayOfWeek.Friday && fridayTag == true)
 {
 i = i + 1;
 }
 else if (day.DayOfWeek == DayOfWeek.Saturday && saturdayTag == true)
 {
 i = i + 1;
 }
 else if (day.DayOfWeek == DayOfWeek.Sunday && sundayTag == true)
 {
 i = i + 1;
 }
 }
 return i > 0 ? i : 0;
}

Answer 1

Using System.Linq library the method can be rewritten like this:

public static int GetRemainingDays(DateTime startDate, DateTime endDate, ISet<DayOfWeek> includedDays)
{
 return Enumerable.Range(0, Int32.MaxValue)
 .Select(n => startDate.AddDays(n+1))
 .TakeWhile(date => date <= endDate)
 .Count(date => includedDays.Contains(date.DayOfWeek));
}

Which can then be more easily called like this:

GetRemainingDays(startDate, endDate, 
 new HashSet<DayOfWeek>{DayOfWeek.Wednesday, DayOfWeek.Friday});

Note: You should test this (and the original version) against off-by-one errors.

• svick: "You should test this against off-by-one errors." I think you actually made one: the first n is 1, and you then use n+1, which means the first tested date is strartDate.AddDays(2), which I think is wrong.
• svick: Also, I probably wouldn't bother with ISet. The maximum number of items in that set is 7, so you don't care about O(1) vs. O(n) here.
• svick: One more thing: what is the TakeWhile() trying to accomplish? Doesn't the original Range() already limit the range correctly?
• abuzittin gillifirca: @svick I was in doubt whether my "refactoring" preserved the for loop behavior to the letter, but as you pointed out in your answer the original version is also suspicious so really didn't bother to work out edge cases. As for ISet my first thought was params DayOfWeek[] but I thought it intention hiding. DayOfWeek[] is good though, and less verbose on the call site.As for TakeWhile, I removed redundancy in Range in the edit such that it more clearly mirrors for loop.

Answer 2

Interface

I can see this method being useful for, say, counting the number of times a class will meet if it is scheduled for Mondays, Wednesdays, and Fridays from 2014年01月01日 to 2014年06月01日.

I would expect startDate to be included in the count; it surprises me that the startDate is not counted. It's not obvious whether endDate should be included or not. Either way, you need to document how the two dates at each end are treated.

This method is a pure function, and should probably be made static.

There is a maxim among programmers that there are either zero, one, or many of something. If you ever need to handle three or seven of something, you should generalize to handle many. By that principle, the method signature should be

public static int CountOccurrences(DateTime startDate, DateTime endDate, DayOfWeek[] days)

One way to implement that is to sum the results of calling a helper method

public static int CountOccurrences(DateTime startDate, DateTime endDate, DayOfWeek day)

... for each of the days.

Implementation

The return statement should be just return i; since i will only be negative in case of overflow.

Your implementation doesn't scale very well if the date range is large. You should find a way to count the number of whole weeks within the range, then handle the partial weeks at each end.

• svick: That CountOccurrences for a single day is an interesting idea, especially since it wouldn't be that hard to implement it in O(1) (i.e. without loops). Though that's probably the premature optimizer in me talking.

Answer 3

Since DayOfWeek is a an Enum with 0 = Sunday through 6 = Saturday.

Instead of passing Boolean tags, you can pass an array of integers where index = 0 is for Sunday, with value 1 for true and 0 for false.

 var dayOfWeeksTags = new int[7];

then you can compute something like

 i += dayOfWeeksTags[(int) day.DayOfWeek];

Your function becomes something like this.

 public int GetRemainingDays(DateTime startDate, DateTime endDate, int[] dayOfWeekTags)
 {
 int i = 0;
 for (DateTime day = startDate.AddDays(1); day.Date <= endDate.Date; day = day.AddDays(1))
 i += dayOfWeekTags[(int) day.DayOfWeek];
 return i;
 }