# Calculate next working day

I have tried to tackle this question as follows:

The program Calculates the next Working day, taking into account:
- weekends (Saturdays and Sundays)
- holidays happening in the middle of the week

param name="OrderDate" the date on which the order is placed
param name="workingDays" the number of workingdays to process the order
returns the date on which the Order is Requested to Ship

    public static DateTime getNextWorkingDay(DateTime OrderDate, int workingDays)
{
DateTime Holiday = new DateTime();
List<DateTime> SelectedHolidays = new List<DateTime>();
List<HolidayList> HolidayList = new List<HolidayList>();

int calculatedWorkingDays=0;
if (workingDays <= 5)
calculatedWorkingDays = workingDays;
else if (workingDays % 6 == 0)
{
calculatedWorkingDays = workingDays + workingDays / 6 * 2;
}
else
{
calculatedWorkingDays = workingDays + workingDays / 7 * 2;
}

HolidayList =  "Read the list from Json file";

foreach (var holiday in HolidayList)
{
string year = DateTime.Today.Year.ToString();
Holiday = DateTime.Parse(holiday.MonthAndDayValue + "-" + year);

if (Holiday.Day == 1 && Holiday.Month == 1)
{
year = (DateTime.Today.Year + 1).ToString();
Holiday = DateTime.Parse(holiday.MonthAndDayValue + "-" + year);
}

if (Holiday.DayOfWeek == DayOfWeek.Sunday || Holiday.DayOfWeek == DayOfWeek.Saturday)
{
//do nothing
}
else
{
if (Holiday.Date == OrderDate.Date)
if (Holiday.Date > OrderDate.Date)
{
int K = (int)(Holiday.Date - OrderDate.Date).TotalDays;
if (K <= calculatedWorkingDays)
{
calculatedWorkingDays++;
}
}
}
}
if (OrderDate.DayOfWeek == DayOfWeek.Sunday)
{
calculatedWorkingDays++;
}
else if (OrderDate.DayOfWeek == DayOfWeek.Saturday || OrderDate.DayOfWeek == DayOfWeek.Friday)
{
calculatedWorkingDays += 2;
}

// this is required date

//check if the last date fall on a non working day(holiday  or weekend)
calculatedWorkingDays = 0;
foreach (DateTime d in SelectedHolidays)
if (d.Date == date_OrderRequestedToShip.Date)
calculatedWorkingDays++;
if (date_OrderRequestedToShip.DayOfWeek == DayOfWeek.Sunday)
{
calculatedWorkingDays = 1;
}
else if (date_OrderRequestedToShip.DayOfWeek == DayOfWeek.Saturday)
{
calculatedWorkingDays = 2;
}

}


The holiday list contains the month and day (MM/dd) part only with out the year part:

{
"holidayName": "Memorial Day",
"MonthAndDayValue": "05-30"
}


Can anyone help to tune up or suggest changes to make the code more efficient?

• Does it work though? If I enter 2015-03-13 as orderdate with 3 working days and I use the 15th, 16th and 17th as holidays it tells me that the 18th is the next working day. But then I've only got the 14th as a tru Mar 9, 2015 at 18:10

First, you should separate a process of parsing your input from your business logic. So, your first step is to convert your holidays input to a structure you'll be working with. So, let's say you have a List<Holiday> where

public struct Holiday
{

public Holiday(int month, int day)
{
Month = month;
Day = day;
}
}


static IEnumerable<DateTime> GetWorkingDays(DateTime startDate, List<Holiday> holidays)
{
var date = startDate;

{
if (date.DayOfWeek != DayOfWeek.Saturday &&
date.DayOfWeek != DayOfWeek.Sunday &&
holidays.All(holiday => holiday.Day != date.Day ||
holiday.Month != date.Month))
{
yield return date;
}
}
}


Note that this iterator is endless, so you should be cautious enumerating it.

Now, your problem can be solved with just one line

var result = GetWorkingDays(orderDate, holidays).Skip(workingDays).First();


Complete example here: https://dotnetfiddle.net/W3dalr

Your method does too much and that makes it hard to follow/optimize.

This should be extracted into its own method.

    int calculatedWorkingDays=0;
if (workingDays <= 5)
calculatedWorkingDays = workingDays;
else if (workingDays % 6 == 0)
{
calculatedWorkingDays = workingDays + workingDays / 6 * 2;
}
else
{
calculatedWorkingDays = workingDays + workingDays / 7 * 2;
}


Then this block

    HolidayList =  "Read the list from Json file";

foreach (var holiday in HolidayList)
{
string year = DateTime.Today.Year.ToString();
Holiday = DateTime.Parse(holiday.MonthAndDayValue + "-" + year);

if (Holiday.Day == 1 && Holiday.Month == 1)
{
year = (DateTime.Today.Year + 1).ToString();
Holiday = DateTime.Parse(holiday.MonthAndDayValue + "-" + year);
}

if (Holiday.DayOfWeek == DayOfWeek.Sunday || Holiday.DayOfWeek == DayOfWeek.Saturday)
{
//do nothing
}
else
{
if (Holiday.Date == OrderDate.Date)
if (Holiday.Date > OrderDate.Date)
{
int K = (int)(Holiday.Date - OrderDate.Date).TotalDays;
if (K <= calculatedWorkingDays)
{
calculatedWorkingDays++;
}
}
}
}


You actually might want to split that into several methods.

Also, watch your white space. There's never a need for more than a single blank line.

Just some points:

    if (workingDays <= 5)
calculatedWorkingDays = workingDays;
else if (workingDays % 6 == 0)
{
calculatedWorkingDays = workingDays + workingDays / 6 * 2;
}
else
{
calculatedWorkingDays = workingDays + workingDays / 7 * 2;
}


Always put braces around your if statements. If not, horrible bugs may occur when you change your code:

For example, lets say you have this if statement:

if(isTrue())
doSomething();


And you want to add doSomething2() to it. You do:

if(isTrue())
doSomething();
doSomething2();


Note that the code above would be equivalent to:

if(isTrue()) {
doSomething();
}
doSomething2();


Which is probably not what you want. But if you put braces:

if(isTrue()) {
doSomething();
doSomething2();
}


Then it is fine.

    // ...
else
{
calculatedWorkingDays = workingDays + workingDays / 7 * 2;
}

HolidayList =  "Read the list from Json file";
// ...


Don't put too much spaces in your code. It makes it look messier and harder to read.

        if (Holiday.DayOfWeek == DayOfWeek.Sunday || Holiday.DayOfWeek == DayOfWeek.Saturday)
{
//do nothing
}
else
{
// ...
}


The empty if can be easily avoided by doing:

        if (!(Holiday.DayOfWeek == DayOfWeek.Sunday || Holiday.DayOfWeek == DayOfWeek.Saturday))
{
// ...
}


    foreach (DateTime d in SelectedHolidays)
if (d.Date == date_OrderRequestedToShip.Date)
calculatedWorkingDays++;


Again, braces:

    foreach (DateTime d in SelectedHolidays) {
if (d.Date == date_OrderRequestedToShip.Date) {
calculatedWorkingDays++;
}
}

• "if statement's braces should start on the same line (as a convention)" This is absolutely not true. Perhaps this is the convention at your place, but I've always written curly braces on a new line. Consistency is important, however: don't mix styles. Mar 9, 2015 at 18:19
• @BCdotWEB thanks for the point, I will remove that from my answer. Mar 9, 2015 at 18:20

There are a few things you can improve. First, never have empty if statements:

if (Holiday.DayOfWeek == DayOfWeek.Sunday || Holiday.DayOfWeek == DayOfWeek.Saturday)
{
//do nothing
}
else
{
if (Holiday.Date == OrderDate.Date)
if (Holiday.Date > OrderDate.Date)
{
int K = (int)(Holiday.Date - OrderDate.Date).TotalDays;
if (K <= calculatedWorkingDays)
{
calculatedWorkingDays++;
}
}
}


Reverse the conditional and get rid of the else:

if (Holiday.DayOfWeek != DayOfWeek.Sunday && Holiday.DayOfWeek != DayOfWeek.Saturday)
{
if (Holiday.Date == OrderDate.Date)
if (Holiday.Date > OrderDate.Date)
{
int K = (int)(Holiday.Date - OrderDate.Date).TotalDays;
if (K <= calculatedWorkingDays)
{
calculatedWorkingDays++;
}
}
}


Also, keep your style similar. In the above, you have a if statement with one line without braces, and another with braces. You should always choose one style, preferably braces, and stick to it; the same applies to loops:

foreach (DateTime d in SelectedHolidays)
if (d.Date == date_OrderRequestedToShip.Date)
calculatedWorkingDays++;


foreach (DateTime d in SelectedHolidays)
{
if (d.Date == date_OrderRequestedToShip.Date)
{
calculatedWorkingDays++;
}
}


Right here, you only use the variable K once:

int K = (int)(Holiday.Date - OrderDate.Date).TotalDays;
if (K <= calculatedWorkingDays)
{
calculatedWorkingDays++;
}


Not only is this a bad and undescriptive name for a variable, it is unnecessary. Also, what type does the .TotalDays return, a double? If so, you probably do not need to cast it to an int.

if ((Holiday.Date - OrderDate.Date).TotalDays <= calculatedWorkingDays)
{
calculatedWorkingDays++;
}


Don't put the braces on the same line with the statement, ie:

if(...) {
}


if (...)