Extract incomplete weeks from a DateTime collection

Question

The code below attempts to extract incomplete weeks from a List<DateTime>.

For example, a list containing all the days in Jan 2015 would result in the 5th to the 25th inclusive.

I know the list going in will contain unique dates, and are in date order. It seems to be working but I can't help but think this could be done better.

private IEnumerable<DateTime> extractIncompleteWeeks(IEnumerable<DateTime> dates)
{
 var mondays = dates
 .Where((d, i) => d.DayOfWeek == DayOfWeek.Monday)
 .Select(d => d.Date);
 var results = new List<DateTime>();
 foreach (var monday in mondays)
 {
 var fullWeek = new HashSet<DateTime>(dates.SkipWhile(d => d < monday).Take(7).Select(d => d.Date));
 if (fullWeek.Last().Date == monday.AddDays(6).Date)
 results.AddRange(fullWeek);
 }
 return results;
}

• BCdotWEB: Is your description accurate? Because this reminds me of a StackOverflow question from a while back.
• ETFairfax: @BCdotWEB: No, I don't think that's quite the same. I need to get all dates where the week is complete. So if there is a wednesday missing in the list passed in, I won't get any dates from that week. Only dates where all "siblings" (mon to sun) are present will be returned.

Answer 1

Naming:

Names of methods in C# use PascalCasing. Your method name would become ExtractIncompleteWeeks. Although the naming convention is now correct, the name is ambiguous. Extract incomplete weeks means you want to fetch only the dates of the weeks which are incomplete. Instead rename it to ChopIncompleteWeeks or ExtractCompleteWeeks. This makes clear that you want to eliminate the incomplete weeks or want to extract the complete weeks.

Return type:

The return type of the method is an IEnumerable<DateTime> yet you return a List<DateTime>. Either make the return type also List<DateTime> but preferably return one element at a time by using yield .

private IEnumerable<DateTime> ExtractCompleteWeeks(IEnumerable<DateTime> dates)
{
 ...
 foreach (var monday in mondays)
 {
 ...
 if (fullWeek.Last().Date == monday.AddDays(6).Date)
 {
 foreach(var day in fullWeek)
 {
 yield return day;
 }
 }
 }
}

Here are some questions from StackOverflow and Programmers.SE regarding yield:

• Proper Use of yield return
• A practical use of "yield" keyword in C#

I really like this answer from the first question to get a very basic understanding of using yield instead of a temporary list:

Populating a temporary list is like downloading the whole video, whereas using yield is like streaming that video.

HashSet:

Is there a specific reason why you create a HashSet<DateTime> only to take the last element and compare it against monday.AddDays(6).Date? You can replace that line by following:

var fullWeek = dates.SkipWhile(d => d < monday).Take(7).Select(d => d.Date);

Complete code:

private IEnumerable<DateTime> ExtractCompleteWeeks(IEnumerable<DateTime> dates)
{
 var mondays = dates.Where(d => d.DayOfWeek == DayOfWeek.Monday)
 .Select(d => d.Date);
 foreach (var monday in mondays)
 {
 var fullWeek = dates.SkipWhile(d => d < monday).Take(7).Select(d => d.Date);
 if (fullWeek.Last().Date == monday.AddDays(6).Date)
 {
 foreach(var day in fullWeek)
 {
 yield return day;
 }
 }
 }
}

• ETFairfax: I've always named my private methods with a starting lower case. Is that "wrong"? I've not really used the yield before - I shall have a read up on it! Many thanks for all the suggestions.
• BCdotWEB: @ETFairfax PascalCase is standard for methods.
• Abbas: It is not wrong in the sense of runtime/compile errors but aesthetically it is wrong if you want to follow Microsoft's naming guidelines. I updated my answer a bit where I talk about yield to give you a simple understanding on it.
• ETFairfax: @Abbas Thanks for the update - that streaming analogy helped a lot.