Extracting opening and closing times for each day of the week

Question

I have these two methods:

private static OpeningDay GetUsallyOpeningHour(List<OpeningDay> openingHoursDaily, DayOfWeek dayOfWeek)
{
    var listOfSpecificDayOfWeek = openingHoursDaily.Where(x => x.DateTime.DayOfWeek == dayOfWeek);

    if(listOfSpecificDayOfWeek.Any())
        return listOfSpecificDayOfWeek.GroupBy(x => x.From).OrderByDescending(x => x.Count()).ThenByDescending(x => x.Key).First().First();

    return new OpeningDay { From = new TimeSpan(0, 0, 0), To = new TimeSpan(0, 0, 0), DateTime = DateTime.Now };
}

private static OpeningDay GetUsallyClosingHour(List<OpeningDay> openingHoursDaily, DayOfWeek dayOfWeek)
{
    var listOfSpecificDayOfWeek = openingHoursDaily.Where(x => x.DateTime.DayOfWeek == dayOfWeek);

    if (listOfSpecificDayOfWeek.Any())
        return listOfSpecificDayOfWeek.GroupBy(x => x.To).OrderByDescending(x => x.Count()).ThenByDescending(x => x.Key).First().First();

    return new OpeningDay { From = new TimeSpan(0, 0, 0), To = new TimeSpan(0, 0, 0), DateTime = DateTime.Now };
}

The only difference between them is that one is grouping by x.From and other by x.To, and I was wondering how I can improve this code.

Basically, I can a create method which takes lambda, but how long? At first I was thinking to pass this:

GroupBy(x => x.To).OrderByDescending(x => x.Count()).ThenByDescending(x => x.Key)

as a lambda parameter. But this would take me ending with function that takes a lambda and then returns sth like lambda.First().First().

What do you recommend? Are there some other improvements?

Answer 1

Your thinking is good: it's a small difference but... there is a difference. And you have to find a way to extract the difference out of the methods and try create a parameter from this. Small example:

public class User
{
    public int Id { get; set; }
    public string Name { get; set; }
    public int Value { get; set; }
}

We could then group them by Id or Value for example:

public Item GetItemGroupedByName(List<Item> items)
{
    return items.GroupBy(x => x.Id).First().First();
}

public Item GetItemGroupedByValue(List<Item> items)
{
    return items.GroupBy(x => x.Value).First().First();
}

This gives the same kind of situation like yours. Luckily there's the Func<T, TResult> Delegate. The method can now look like this:

public Item GetItemGrouped(List<Item> items, Func<Item, int> rule)
{
    return items.GroupBy(rule).First().First();
}

And you call it like this:

GetItemGrouped(list, x => x.Value);
//or:
GetItemGrouped(list, x => x.Id);

Of course you can't group by Name in this case as it is a string property and the Func expects an integer. You might change the int to object, but please don't as that is hacky code. Since the purpose of your method is to group by a TimeSpan, you can implement this easily:

private static OpeningDay GetUsallyOpeningHour(List<OpeningDay> openingHoursDaily,
                                               DayOfWeek dayOfWeek,
                                               Func<OpeningDay, TimeSpan> groupByRule)
{
    var listOfSpecificDayOfWeek = openingHoursDaily.Where(x => x.DateTime.DayOfWeek == dayOfWeek);

    if(listOfSpecificDayOfWeek.Any())
        return listOfSpecificDayOfWeek.GroupBy(groupByRule)
                                      .OrderByDescending(x => x.Count())
                                      .ThenByDescending(x => x.Key)
                                      .First().First();

    return new OpeningDay { From = new TimeSpan(0, 0, 0), To = new TimeSpan(0, 0, 0), DateTime = DateTime.Now };
}

And now you can call the method like:

var openingDay = GetUsallyOpeningHour(openingHours, DayOfWeek.Thursday, x => x.From);
//or:
var openingDay = GetUsallyOpeningHour(openingHours, DayOfWeek.Thursday, x => x.To);