Good Ole Refactoring

I was talking with some co-workers about a pull request recently and we all decided we didn’t really like the look of some code.

Here's the code:

	private DateTime GetBusinessDayByNumberOfDaysFromCurrentDate(int daysAgo)
	{
	var previousDay = SystemClock.UtcNow;
	var daysToGoBack = daysAgo;
	while (IsHoliday(previousDay) || IsDayOnWeekend(previousDay) || daysToGoBack > 0)
	{
	previousDay = previousDay.AddDays(-1);
	var businessDay = !IsHoliday(previousDay) && !IsDayOnWeekend(previousDay);
	if (!businessDay) { continue; }
	daysToGoBack--;
	}
	return previousDay;
	}

view raw GetBusinessDaysByNumberOfDaysFromCurrentDate hosted with ❤ by GitHub

Our main issue with the code was that it was difficult to read and immediately understand what was going on.

Here are the things you should know:

• The SystemClock is a Headspring nuget package
• IsDayOnWeekend just checks if the current day is a Saturday or Sunday
• IsHoliday checks whether or not the day falls on a holiday

So we did a small refactoring to make the code more readable.

	private DateTime GetPreviousBusinessDayByNumberOfDaysFromCurrentDate(int daysAgo)
	{
	var lastThreeHundredSixtyFiveDays = Enumerable.Range(0, 365).Select(n => SystemClock.UtcNow.Date.AddDays(-n));
	var validBusinessDays = lastThreeHundredSixtyFiveDays.Where(date => !date.IsDayOnWeekend() && !date.IsHoliday(_companyHolidays));
	var lastBusinessDayByNumber = validBusinessDays.OrderByDescending(date => date).Skip(daysAgo - 1).First();
	return lastBusinessDayByNumber;
	}

view raw GetPreviousBusinessDayByNumberOfDaysFromCurrentDate hosted with ❤ by GitHub

• So we renamed the method to include GetPreviousBusinessDay
• We get the last 365 days, we could've just as easily gotten the last 30, but decided to just get the full year.
• After we get the last 365 days, we get only the valid business days
• Then we order those dates descendingly, skip the number of days back we want to go, and then grab the first date

Our refactoring is more readable, has some "clever" code that we like, but aren't married to either. I'm curious to know other opinions, and other solutions that we could've used.