Class for managing blocks of working hours

Question

I have written the following class:

NOTE: There is a bug in the third party component that means that if a DHXBlockTime is there for a day of the week and a specific date then only the ones with a specific date are shown so that is why there is the seemingly redundant AddNonWorkingHoursAsDateInsteadOfDay method.

internal  class CalendarHelper
{
    internal static IEnumerable<DHXBlockTime> GetUnavailableBlocks(IEnumerable<Driver> availableDrivers, string viewName)
    {
        var unavailable = new List<DHXBlockTime>();

        foreach (var driver in availableDrivers)
        {
            AddNonWorkingHours(viewName, unavailable, driver);

            foreach (var away in driver.Aways)
            {
                unavailable.Add(new DHXBlockTime()
                {
                    StartDate = away.StartDate,
                    EndDate = away.EndDate,
                    Sections = new List<Section>()
                    {
                        new Section(viewName, new string[] {away.EntityPersonStaffId.ToString()})
                    }
                });

                var startDate = new DateTime(away.StartDate.Year, away.StartDate.Month, away.StartDate.Day);
                var endDate = new DateTime(away.EndDate.Year, away.EndDate.Month, away.EndDate.Day);

                var workingHours = driver.WorkingHours.FirstOrDefault(w => w.Active && w.DayOfWeekIndex == (int)startDate.DayOfWeek);
                AddNonWorkingHoursAsDateInsteadOfDay(viewName, workingHours, unavailable, startDate, away.EntityPersonStaffId);

                if (startDate < endDate)
                {
                    workingHours = driver.WorkingHours.FirstOrDefault(w => w.Active && w.DayOfWeekIndex == (int)endDate.DayOfWeek);
                    AddNonWorkingHoursAsDateInsteadOfDay(viewName, workingHours, unavailable, endDate, away.EntityPersonStaffId);
                }

            }
        }

        return unavailable;
    }

    private static void AddNonWorkingHoursAsDateInsteadOfDay(string viewName, EntityPersonStaffHours workingHours, ICollection<DHXBlockTime> unavailable, DateTime awayDate, int staffId)
    {
        if (workingHours == null)
        {
            AddDefaultUnavailableTimes(viewName, unavailable, awayDate, staffId);
        }
        else
        {
            if (!workingHours.IsWorking)
            {
                unavailable.Add(new DHXBlockTime()
                {
                    StartDate = awayDate,
                    EndDate = awayDate.AddHours(24),
                    Sections = new List<Section>()
                    {
                        new Section(viewName,
                            new string[] {staffId.ToString()})
                    }
                });
            }
            else
            {
                int startMinutes = 7 * 60;
                int endMinutes = 22 * 60;

                if (workingHours.StartTime != null) startMinutes = (int) workingHours.StartTime.Value.TotalMinutes;
                if (workingHours.EndTime != null) endMinutes = (int) workingHours.EndTime.Value.TotalMinutes;

                unavailable.Add(new DHXBlockTime()
                {
                    StartDate = awayDate,
                    EndDate = awayDate.AddMinutes(startMinutes),
                    Sections = new List<Section>()
                    {
                        new Section(viewName, new string[] {staffId.ToString()})
                    }
                });

                unavailable.Add(new DHXBlockTime()
                {
                    StartDate = awayDate.AddMinutes(endMinutes),
                    EndDate = awayDate.AddHours(24),
                    Sections = new List<Section>()
                    {
                        new Section(viewName, new string[] {staffId.ToString()})
                    }
                });

            }
        }
    }

    private static void AddDefaultUnavailableTimes(string viewName, ICollection<DHXBlockTime> unavailable, DateTime awayDate, int staffId)
    {
        unavailable.Add(new DHXBlockTime()
        {
            StartDate = awayDate,
            EndDate = awayDate.AddHours(7),
            Sections = new List<Section>()
            {
                new Section(viewName, new string[] {staffId.ToString()})
            }
        });
        unavailable.Add(new DHXBlockTime()
        {
            StartDate = awayDate.AddHours(22),
            EndDate = awayDate.AddHours(24),
            Sections = new List<Section>()
            {
                new Section(viewName, new string[] {staffId.ToString()})
            }
        });
    }

    private static void AddNonWorkingHours(string viewName, ICollection<DHXBlockTime> unavailable, Driver driver)
    {
        for (int day = 0; day < 7; day++)
        {
            int startOne = 0;
            int endOne = 7 * 60;
            int startTwo = 22 * 60;
            int endTwo = 24 * 60;

            if (driver.WorkingHours.Count > 0)
            {
                var workingHours = driver.WorkingHours.FirstOrDefault(w => w.Active && w.DayOfWeekIndex == day);
                if (workingHours != null)
                {
                    if (workingHours.IsWorking)
                    {
                        if (workingHours.StartTime != null)
                            endOne = (int)workingHours.StartTime.Value.TotalMinutes;
                        if (workingHours.EndTime != null)
                            startTwo = (int)workingHours.EndTime.Value.TotalMinutes;
                    }
                    else
                    {
                        endOne = 12 * 60;
                        startTwo = 12 * 60;
                    }
                }
            }

            var zones = new List<Zone>();
            if (endOne == startTwo)
                zones.Add(new Zone { Start = startOne, End = endTwo });
            else
            {
                zones.Add(new Zone { Start = startOne, End = endOne });
                zones.Add(new Zone { Start = startTwo, End = endTwo });
            }

            unavailable.Add(new DHXBlockTime()
            {
                Day = (DayOfWeek)day,
                Zones = zones,
                Sections = new List<Section>()
                {
                    new Section(viewName, new string[] {driver.ID.ToString()})
                }
            });
        }
    }

}

I am pretty sure that it needs refactored as it is all static methods, but I am not sure how exactly. I think the class should be given a constructor as follows:

private IEnumerable<Driver> _availableDrivers;
private readonly string _viewName;

internal CalendarHelper(IEnumerable<Driver> availableDrivers, string viewName)
{
    _availableDrivers = availableDrivers;
    _viewName = viewName;
}

And then the availableDrivers and viewName parameters are not needed on each method and they can become instance methods rather than static methods, but then I get a class which only has one public method. Another developer has commented that doing this is a needless complication and so I am looking for guidance.

Answer 1

You have a lot of repeating code like this:

unavailable.Add(new DHXBlockTime()
{
    StartDate = away.StartDate,
    EndDate = away.EndDate,
    Sections = new List<Section>()
    {
        new Section(viewName, new string[] {away.EntityPersonStaffId.ToString()})
    }
});

Define a method that will do all this stuff:

private static void AddUnavailableBlockTime(ICollection<DHXBlockTime> unavailable,
                                            DateTime startDate,
                                            DateTime endDate,
                                            string sectionViewName,
                                            int sectionDriverId,
                                            IEnumerable<Zone> zones = null)
{
    unavailable.Add(new DHXBlockTime
    {
        StartDate = startDate,
        EndDate = endDate,
        Zones = zones,
        Sections = new List<Section>
        {
            new Section(sectionViewName, new[] { sectionDriverId.ToString() })
        }
    });
}

Then you can use it like this:

private static void AddDefaultUnavailableTimes(string viewName, ICollection<DHXBlockTime> unavailable, DateTime awayDate, int staffId)
{
    AddUnavailableBlockTime(unavailable,
                            awayDate,
                            awayDate.AddHours(7),
                            viewName,
                            staffId);
    AddUnavailableBlockTime(unavailable,
                            awayDate.AddHours(22),
                            awayDate.AddHours(24),
                            viewName,
                            staffId);
}

As for this code:

int startOne = 0;
int endOne = 7 * 60;
int startTwo = 22 * 60;
int endTwo = 24 * 60;

you should definitely extract all this values to named constants since it is unclear what these magic numbers mean.

And finally I recommend to change List<DHXBlockTime> everywhere in your code on IEnumerable<DHXBlockTime> and use yield return to produce result collection. It is not good to pass changeable collection between multiple methods. For example AddUnavailableBlockTime will be turned into

private static DHXBlockTime GetUnavailableBlockTime(DateTime startDate,
                                                    DateTime endDate,
                                                    string sectionViewName,
                                                    int sectionDriverId,
                                                    IEnumerable<Zone> zones = null)
{
    return new DHXBlockTime
    {
        StartDate = startDate,
        EndDate = endDate,
        Zones = zones,
        Sections = new List<Section>
        {
            new Section(sectionViewName, new[] { sectionDriverId.ToString() })
        }
    };
}

and AddDefaultUnavailableTimes will become

private static IEnumerable<DHXBlockTime> GetDefaultUnavailableTimes(string viewName, DateTime awayDate, int staffId)
{
    yield return GetUnavailableBlockTime(awayDate,
                                         awayDate.AddHours(7),
                                         viewName,
                                         staffId);
    yield return AddUnavailableBlockTime(awayDate.AddHours(22),
                                         awayDate.AddHours(24),
                                         viewName,
                                         staffId);
}