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

Implementing a week schedule class in C#

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

Problem

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.



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

-
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(...)).

-
Is the HashCode function ok for Time class?


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

```
class WeekSchedule
{
private readonly Dictionary activeDaysDict;
private readonly bool isActive247;

public WeekSchedule(IEnumerable days, TimeRange weekdayHours, TimeRange weekendHours)
{
Debug.Assert(days.Count() != 0);
activeDaysDict = new Dictionary();
foreach

Solution

Reactions on your points:

  • The extra classes for Time, TimeSpan, etc. make it easier to read


and use, although you have some extra code.

  • 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.



  • 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 && endDate <= range._endDate;
    }
}

Code Snippets

public override string ToString() => $"{_startDate} - {_endDate}";
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;
    }
}

Context

StackExchange Code Review Q#157458, answer score: 4

Revisions (0)

No revisions yet.