HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Calculate working days and excluding specific dates (Holidays)

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
datesexcludingworkingcalculatespecificandholidaysdays

Problem

I read some questions/answers related to functions in C# to calculate the working days; some use an extended code to achieve that.

I have a data table with more than 50,000 rows and I required an effective method to calculate this information.

I created the following extension in C# that return a number of days (Integer), between two dates; excluding dates (Based on a list of DateTime)

///  Get working days between two dates (Excluding a list of dates - Holidays) 
/// Current date time
/// Finish date time
/// List of dates to exclude (Holidays)
public static int fwGetWorkingDays(this DateTime dtmCurrent, DateTime dtmFinishDate, List lstExcludedDates)
{
    Func workDay = currentDate =>
    (
        currentDate.DayOfWeek == DayOfWeek.Saturday ||
        currentDate.DayOfWeek == DayOfWeek.Sunday ||
        lstExcludedDates.Exists(evalDate => evalDate.Date.Equals(currentDate.Date))
    );

    return Enumerable.Range(0, 1 + (dtmFinishDate - dtmCurrent).Days).Count(intDay => workDay(dtmCurrent.AddDays(intDay)));
}


Does there exists some way to improve this method.

Solution

Bug

There is a bug in this method. It counts non-working days because the workDay condition needs to be negated.

return 
    Enumerable
        .Range(0, (finishDate - current).Days)
        .Count(day => !isExcludedDate(current.AddDays(day)));


Naming

Then comes the naming. What's with the prefixes fw, dtm, lst? In C# we don't use the hungarian notation. You should use either the camelCase or PascalCase naming where appropriate.

Closures

You could take advantage of closures and simplify the Count

public static int GetWorkingDays(this DateTime current, DateTime finishDateExclusive, List excludedDates)
{
    Func isWorkingDay = days =>
    {
        var currentDate = current.AddDays(days);
        var isNonWorkingDay =
            currentDate.DayOfWeek == DayOfWeek.Saturday ||
            currentDate.DayOfWeek == DayOfWeek.Sunday ||
            excludedDates.Exists(excludedDate => excludedDate.Date.Equals(currentDate.Date));
        return !isNonWorkingDay;
    };
   
    return Enumerable.Range(0, (finishDateExclusive - current).Days).Count(isWorkingDay);
}


Depending on how long the List excludedDates is, you may consider using a HashSet.
Upper range

1 + (dtmFinishDate - dtmCurrent).Days


Most .NET APIs consider the upper range exclusive thus it might be a good idea to follow this style by removig the +1 and renaming the finishDate to finishDateExclusive.

Code Snippets

return 
    Enumerable
        .Range(0, (finishDate - current).Days)
        .Count(day => !isExcludedDate(current.AddDays(day)));
public static int GetWorkingDays(this DateTime current, DateTime finishDateExclusive, List<DateTime> excludedDates)
{
    Func<int, bool> isWorkingDay = days =>
    {
        var currentDate = current.AddDays(days);
        var isNonWorkingDay =
            currentDate.DayOfWeek == DayOfWeek.Saturday ||
            currentDate.DayOfWeek == DayOfWeek.Sunday ||
            excludedDates.Exists(excludedDate => excludedDate.Date.Equals(currentDate.Date));
        return !isNonWorkingDay;
    };
   
    return Enumerable.Range(0, (finishDateExclusive - current).Days).Count(isWorkingDay);
}
1 + (dtmFinishDate - dtmCurrent).Days

Context

StackExchange Code Review Q#161135, answer score: 7

Revisions (0)

No revisions yet.