3
\$\begingroup\$

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?

\$\endgroup\$
0

1 Answer 1

2
\$\begingroup\$

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);
\$\endgroup\$
4
  • 2
    \$\begingroup\$ With items.GroupBy(x => x.Id).First().First();, isn't that the same as items.OrderBy(x => x.Id).First();? \$\endgroup\$
    – Nick Udell
    Commented May 26, 2015 at 9:01
  • 1
    \$\begingroup\$ @NickUdell Yes indeed it is, didn't think of it until you mentioned it. :) \$\endgroup\$
    – Abbas
    Commented May 26, 2015 at 9:24
  • \$\begingroup\$ @Abbas but in my example you can't just repleace GroupBy with OrderBy , it would return different result. \$\endgroup\$
    – kosnkov
    Commented May 26, 2015 at 14:47
  • \$\begingroup\$ @kosnkov That is why I didn't change the code in my answer as in some cases (my simple example) it might return the same and in other (more complex like yours) it might not return the same result. :) \$\endgroup\$
    – Abbas
    Commented May 26, 2015 at 14:49

Your Answer

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

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