5
\$\begingroup\$

Requirements

A week schedule class

.NET framework has DateTime struct, but I want a general week schedule class that involves day of week instead of date. For example, a shop has open hours from 9:00AM to 5:00PM Monday through Friday, and from 11:00AM to 4:00PM on Saturday and Sunday.

A massage method

An important method I need from this class is, given a start date of DateTime type and a time span of TimeSpan type, it massages them into the week schedule and outputs DateTime blocks in order that falls into the schedule. For example, consider the shop described above and start date of 3/10/2017 10AM, time span of 15 hours, the method should return the following time blocks as a list.

(3/10/2017 10AM, 3/10/2017 5PM)
(3/11/2017 11AM, 3/11/2017 4PM)
(3/12/2017 11AM, 3/12/2017 2PM)

You can tell the total time in the time blocks are 15 hours, which matches the time span input parameter.


My work so far

Here is what I have so far. But I have a lot of questions and doubts.

  1. Breaking things down to Time, TimeRange, DayTimeRange, WeekSchedule is a good idea of not? If not, what could be a better alternative?

  2. Should the Time/TimeRange/DayTimeRange/WeekSchedule classes be struct or not? Just like the DateTime structs so that when we create a new DateTime, there is constructor where you can put year, month, day, hour, minute, second, TimeRange can have a constructor for start hour, start minute, start second, end hour, end minute and end second. Thus, I do not have to do new TimeRange(new Time(...), new Time(...)).

  3. Is the HashCode function ok for Time class?

Well, any comment, feedback and suggestion is very much appreciated!

   class WeekSchedule
   {
    private readonly Dictionary<DayOfWeek, DayTimeRange> activeDaysDict;
    private readonly bool isActive247;

    public WeekSchedule(IEnumerable<DayOfWeek> days, TimeRange weekdayHours, TimeRange weekendHours)
    {
        Debug.Assert(days.Count() != 0);
        activeDaysDict = new Dictionary<DayOfWeek, DayTimeRange>();
        foreach (var day in days)
        {
            if (IsWeekDay(day))
            {
                if (!activeDaysDict.ContainsKey(day))
                {
                    activeDaysDict.Add(day, new DayTimeRange(day, weekdayHours));
                }
            }
            else
            {
                if (!activeDaysDict.ContainsKey(day))
                {
                    activeDaysDict.Add(day, new DayTimeRange(day, weekendHours));
                }
            }
        }
    }

    public bool Contains(DateTime datetime)
    {
        if (isActive247)
        {
            return true;
        }

        DayTimeRange dayTimeRange;
        return activeDaysDict.TryGetValue(datetime.DayOfWeek, out dayTimeRange) && dayTimeRange.Contains(datetime);
    }

    public IEnumerator<DateRange> GetDateTimeRangeBlockEnumerator(DateTime startDateTime, TimeSpan timeSpan)
    {
        if (isActive247)
        {
            yield return new DateRange(startDateTime, startDateTime + timeSpan);
        }

        while (timeSpan.TotalSeconds > 0)
        {
            bool foundNext = false;
            TimeSpan smallerTimeSpan = TimeSpan.Zero;

            while (!foundNext)
            {
                DayTimeRange dayTimeRange;
                if (activeDaysDict.TryGetValue(startDateTime.DayOfWeek, out dayTimeRange))
                {
                    var res = dayTimeRange.compareTo(startDateTime);
                    if (res == 0)
                    {
                        var remainingDayTimeSpan = dayTimeRange.EndTime.OverrideTime(startDateTime) - startDateTime;
                        smallerTimeSpan = timeSpan < remainingDayTimeSpan ? timeSpan : remainingDayTimeSpan;
                        foundNext = true;
                    }
                    else if (res > 0)
                    {
                        startDateTime = dayTimeRange.StartTime.OverrideTime(startDateTime);
                        var remainingDayTimeSpan = dayTimeRange.GetTimeSpan();
                        smallerTimeSpan = timeSpan < remainingDayTimeSpan ? timeSpan : remainingDayTimeSpan;
                        foundNext = true;
                    }
                    else
                    {
                        // move to the beginning of next date
                        startDateTime = startDateTime.AddDays(1).Date;
                    }
                }
                else
                {
                    startDateTime = startDateTime.AddDays(1).Date;
                }
            }

            yield return new DateRange(startDateTime, smallerTimeSpan);
            startDateTime = startDateTime.AddDays(1).Date;
            timeSpan -= smallerTimeSpan;
        }
    }

    private bool IsWeekDay(DayOfWeek day)
    {
        return day != DayOfWeek.Saturday && day != DayOfWeek.Sunday;
    }
}

class DateRange
{
    private readonly DateTime _startDate;
    private readonly DateTime _endDate;
    private readonly double _totalHours;

    public DateTime StartDate { get { return _startDate; } }
    public DateTime EndDate { get { return _endDate; } }

    public DateRange(DateTime startDate, DateTime endDate)
    {
        if (startDate >= endDate)
        {
            throw new Exception("Start DateTime is no early than End DateTime.");
        }
        _startDate = startDate;
        _endDate = endDate;
        _totalHours = (_endDate - _startDate).TotalHours;
    }

    public DateRange(DateTime startDate, TimeSpan span)
    {
        if (span <= TimeSpan.Zero)
        {
            throw new Exception("Start DateTime is no early than End DateTime.");
        }

        _startDate = startDate;
        _endDate = startDate + span;
        _totalHours = span.TotalHours;
    }

    public double TotalHours { get { return _totalHours; } }

    public TimeSpan GetOverlappingTime(DateRange range)
    {
        if (_startDate >= range._endDate || _endDate <= range._startDate) { return TimeSpan.Zero; }
        if (_startDate >= range._startDate && _endDate <= range._endDate) { return _endDate - _startDate; }
        if (_startDate <= range._startDate && _endDate >= range._endDate) { return range._endDate - range._startDate; }
        if (_startDate > range._startDate && _startDate < range._endDate) { return range._endDate - _startDate; }
        return _endDate - range._startDate;
    }

    public bool IsDateInRange(DateTime date)
    {
        return (date >= _startDate) && (date < _endDate);
    }

    public override string ToString()
    {
        return _startDate.ToString() + " - " + _endDate.ToString();
    }
}

class DayTimeRange
{
    private readonly DayOfWeek day;
    private readonly TimeRange timeRange;

    public Time StartTime { get { return timeRange.StartTime; } }
    public Time EndTime { get { return timeRange.EndTime; } }

    public DayTimeRange(DayOfWeek day, TimeRange timeRange)
    {
        this.day = day;
        this.timeRange = timeRange;
    }

    public TimeSpan GetTimeSpan()
    {
        return timeRange.GetTimeSpan();
    }

    public bool Contains(DateTime datetime)
    {
        if (datetime.DayOfWeek == day)
        {
            var totalSecondsSinceMidnight = (uint)(datetime.Hour * 60 * 60 + datetime.Minute * 60 + datetime.Second);
            if (timeRange.Contains(totalSecondsSinceMidnight))
            {
                return true;
            }
        }

        return false;
    }

    public int? compareTo(DateTime datetime)
    {
        if (datetime.DayOfWeek == day)
        {
            return timeRange.CompareTo(datetime);
        }

        // cannot compare if datetime is not on the same day of the week.
        return null;
    }

    public override string ToString()
    {
        return timeRange.ToString();
    }
}

class TimeRange : IComparable<DateTime>
{
    private readonly Time _startTime;
    private readonly Time _endTime;

    public Time StartTime { get { return _startTime; } }
    public Time EndTime { get { return _endTime; } }

    public TimeRange(Time startTime, Time endTime)
    {
        Debug.Assert(startTime != null);
        Debug.Assert(endTime != null);
        Debug.Assert(startTime < endTime);
        this._startTime = startTime;
        this._endTime = endTime;
    }

    public bool Contains(Time time)
    {
        return time >= _startTime && time <= _endTime;
    }

    public bool Contains(uint totalSeconds)
    {
        return CompareTo(totalSeconds) == 0;
    }

    public int CompareTo(DateTime datetime)
    {
        var totalSecondsSinceMidnight = (uint)(datetime.Hour * 60 * 60 + datetime.Minute * 60 + datetime.Second);
        return CompareTo(totalSecondsSinceMidnight);
    }

    /// <summary>
    /// Compare the Time Range with a total seconds start from midnight.
    /// If input is smaller, then return 1; If it's bigger, return -1. Otherwise, 0
    /// </summary>
    /// <param name="totalSeconds"></param>
    /// <returns></returns>
    public int CompareTo(uint totalSeconds)
    {
        if (_startTime.TotalSeconds > totalSeconds)
        {
            return 1;
        }
        else if (_endTime.TotalSeconds < totalSeconds)
        {
            return -1;
        }

        return 0;
    }

    public TimeSpan GetTimeSpan()
    {
        return new TimeSpan(0, 0, _endTime.TotalSeconds - _startTime.TotalSeconds);
    }

    public override string ToString()
    {
        return _startTime.ToString() + " - " + _endTime.ToString();
    }

}

class Time : IComparable<Time>
{
    private readonly int _hours;
    private readonly int _minutes;
    private readonly int _seconds;
    private readonly int _totalSeconds;

    public int Hours { get { return _hours; } }
    public int Minutes { get { return _minutes; } }
    public int Seconds { get { return _seconds; } }
    public int TotalSeconds { get { return _totalSeconds; } }

    public Time(uint h, uint m, uint s)
    {
        if (h > 23 || m > 59 || s > 59)
        {
            throw new ArgumentException("Invalid time specified");
        }
        _hours = (int)h; _minutes = (int)m; _seconds = (int)s;
        _totalSeconds = _hours * 60 * 60 + _minutes * 60 + _seconds;
    }

    public Time(uint totalSeconds)
    {
        if (totalSeconds >= 24 * 60 * 60)
        {
            throw new ArgumentException("Invalid time specified");
        }

        _totalSeconds = (int)totalSeconds;
        _hours = _totalSeconds / (60 * 60);
        _minutes = _totalSeconds % (60 * 60) / 60;
        _seconds = _totalSeconds - _hours * 60 * 60 - _minutes * 60;

    }

    public Time(DateTime dt)
    {
        _hours = dt.Hour;
        _minutes = dt.Minute;
        _seconds = dt.Second;
    }

    public DateTime OverrideTime(DateTime dt)
    {
        return new DateTime(dt.Year, dt.Month, dt.Day, _hours, _minutes, _seconds);
    }

    public override string ToString()
    {
        return String.Format(
            "{0:00}:{1:00}:{2:00}",
            this._hours, this._minutes, this._seconds);
    }

    public int CompareTo(Time other)
    {
        return TotalSeconds - other.TotalSeconds;
    }

    public static bool operator <(Time t1, Time t2)
    {
        return t1.CompareTo(t2) < 0;
    }

    public static bool operator >(Time t1, Time t2)
    {
        return t1.CompareTo(t2) > 0;
    }

    public static bool operator >=(Time t1, Time t2)
    {
        return t1.CompareTo(t2) >= 0;
    }

    public static bool operator <=(Time t1, Time t2)
    {
        return t1.CompareTo(t2) <= 0;
    }

    public override bool Equals(object obj)
    {
        var item = obj as Time;

        if (item == null)
        {
            return false;
        }

        return this.CompareTo(item) == 0;
    }

    public override int GetHashCode()
    {
        return this.TotalSeconds;
    }
}
\$\endgroup\$

3 Answers 3

4
\$\begingroup\$

Reactions on your points:

  1. The extra classes for Time, TimeSpan, etc. make it easier to read and use, although you have some extra code.
  2. Structs are not used to often, classes have more possibilities. It is better to not have a lot of parameters, so adding an new Time object is better.
  3. I think it can be a bit more complex. See: Why is it important to override GetHashCode when Equals method is overridden?

Overal it looks good. Some points:

  • It would be good to also add some unittests, to see how to use it.

  • Add access modifiers to the classes.

  • Use string interpolation.

    public override string ToString() => $"{_startDate} - {_endDate}";
    
  • Do not use Exception but a more specialised exception or a custom exception.

Update:

  • The if conditions in GetOverlappingTime are a bit hard to read. It would be good to make simple (Extention) methods for those.

Like:

public TimeSpan GetOverlappingTime(DateRange range)
{
    if (range.AreOutOfRange(_startDate, _endDate)) { return TimeSpan.Zero; }
    if (range.AreInRange(_startDate, _endDate)) { return _endDate - _startDate; }
    // ...
 }

public static class Extentions
{
    public static bool AreOutOfRange(this DateRange range, DateTime startDate, DateTime endDate)
    {
        return startDate >= range._endDate || endDate <= range._startDate;
    }

    public static bool AreInRange(this DateRange range, DateTime startDate, DateTime endDate)
    {
        return startDate >= range._startDate && endDate <= range._endDate;
    }
}
\$\endgroup\$
2
\$\begingroup\$

Just a quick shot at the WeekSchedule constructor. There is too much duplicated code. By using the conditional-operator you only need the code to check once if the dict contains the day, like so

public WeekSchedule(IEnumerable<DayOfWeek> days, TimeRange weekdayHours, TimeRange weekendHours)
{
    Debug.Assert(days.Count() != 0);
    activeDaysDict = new Dictionary<DayOfWeek, DayTimeRange>();
    foreach (var day in days)
    {
        var currentTimeRange = IsWeekDay(day) ? weekdayHours : weekendHours;

        if (!activeDaysDict.ContainsKey(day))
        {
            activeDaysDict.Add(day, new DayTimeRange(day, currentTimeRange));
        }
    }
}
\$\endgroup\$
1
  • \$\begingroup\$ It is a conditional-operator instead of a null-conditional-operator, I edited it. \$\endgroup\$ Mar 29, 2018 at 13:55
2
\$\begingroup\$

Time

You store the members as int, but accept them as uint. I would be more consistent. There is a guard condition anyway in the constructor.

public Time(uint h, uint m, uint s)
    {
        if (h > 23 || m > 59 || s > 59)
        {
            throw new ArgumentException("Invalid time specified");
        }
        _hours = (int)h; _minutes = (int)m; _seconds = (int)s;
        _totalSeconds = _hours * 60 * 60 + _minutes * 60 + _seconds;
    }

The method OverrideTime could be renamed to ApplyTo. This way it is clear the instance updates the parameter and not the other way around.

public DateTime OverrideTime(DateTime dt)
    {
        return new DateTime(dt.Year, dt.Month, dt.Day, _hours, _minutes, _seconds);
    }

TimeRange

I would favor operator overloads rather than explicit convert methods like GetTimeSpan. The language enables us to write operators, so why not use them?

public TimeSpan GetTimeSpan()
    {
        return new TimeSpan(0, 0, _endTime.TotalSeconds - _startTime.TotalSeconds);
    }

DayTimeRange

I would not deviate from a common pattern, specially when using its exact name CompareTo. I would rather implement IComparable and return -1 or 1 when the day does not match.

 public int? compareTo(DateTime datetime)
    {
        if (datetime.DayOfWeek == day)
        {
            return timeRange.CompareTo(datetime);
        }

        // cannot compare if datetime is not on the same day of the week.
        return null;
    }

DateRange

Method GetOverlappingTime could be rewritten as

public TimeSpan GetOverlappingTime(DateRange range)
    {
        if (_startDate >= range._endDate // ..
        // ..
        return _endDate - range._startDate;
    }
public TimeSpan GetOverlappingTime(DateRange range)
    {
        var start = range._startDate > _startDate 
             ? range._startDate : _startDate;
        var end = range._endDate < _endDate 
             ? range._enDate : _endDate;
        return end < start ? TimeSpan.Zero : end - start;
    }

General

  • Why choose private classes with Debug asserts instead of public classes with exceptions?
  • The hash code is fine. It should be fast, idempotent and a superset of the identity of the instance.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.