patterncsharpMinor
Extracting opening and closing times for each day of the week
Viewed 0 times
theeachdayweekforextractingandtimesopeningclosing
Problem
I have these two methods:
The only difference between them is that one is grouping by
Basically, I can a create method which takes lambda, but how long? At first I was thinking to pass this:
as a lambda parameter. But this would take me ending with function that takes a lambda and then returns sth like
What do you recommend? Are there some other improvements?
private static OpeningDay GetUsallyOpeningHour(List 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 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?
Solution
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:
We could then group them by
This gives the same kind of situation like yours. Luckily there's the Func Delegate. The method can now look like this:
And you call it like this:
Of course you can't group by
And now you can call the method like:
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 items)
{
return items.GroupBy(x => x.Id).First().First();
}
public Item GetItemGroupedByValue(List items)
{
return items.GroupBy(x => x.Value).First().First();
}This gives the same kind of situation like yours. Luckily there's the Func Delegate. The method can now look like this:
public Item GetItemGrouped(List items, Func 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 openingHoursDaily,
DayOfWeek dayOfWeek,
Func 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);Code Snippets
public class User
{
public int Id { get; set; }
public string Name { get; set; }
public int Value { get; set; }
}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();
}public Item GetItemGrouped(List<Item> items, Func<Item, int> rule)
{
return items.GroupBy(rule).First().First();
}GetItemGrouped(list, x => x.Value);
//or:
GetItemGrouped(list, x => x.Id);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 };
}Context
StackExchange Code Review Q#91754, answer score: 2
Revisions (0)
No revisions yet.