C# Switch Statement refactoring

Question

The purpose of the code below is to determine if a particular date qualifies as a "weekend" i.e after 12:00 PM on Thursday, minimum 2 days and before Monday 12:00 PM

Is there a better way? If-Else turns ugly and the Strategy pattern is way too much work for this.

public bool ValidateWeekend(DateTime pickupDate, DateTime dropoffDate)
    {
        TimeSpan ts = dropoffDate.Subtract(pickupDate);

        if (ts.TotalDays >= 2 && ts.TotalDays <= 4)
        {
            switch (pickupDate.DayOfWeek)
            {
                case DayOfWeek.Thursday:
                    if (pickupDate.Hour >= 12)
                    {
                        switch (dropoffDate.DayOfWeek)
                        {
                            case DayOfWeek.Sunday:
                                return true;
                            case DayOfWeek.Monday:
                                if (dropoffDate.Hour <= 12)
                                {
                                    return true;
                                }
                                return false;
                        }
                    }
                    break;
                case DayOfWeek.Friday:
                    switch (dropoffDate.DayOfWeek)
                    {
                        case DayOfWeek.Sunday:
                            return true;
                        case DayOfWeek.Monday:
                            if (dropoffDate.Hour <= 12)
                            {
                                return true;
                            }
                            return false;
                    }
                    break;
                case DayOfWeek.Saturday:
                    switch (dropoffDate.DayOfWeek)
                    {
                        case DayOfWeek.Sunday:
                            return true;
                        case DayOfWeek.Monday:
                            if (dropoffDate.Hour <= 12)
                            {
                                return true;
                            }
                            return false;
                    }
                    return false;
            }
        }
        return false;
    }

In addition to refactoring duplicated code into its own method as listed below, I'm a big fan of using a more functional approach to the solution. Isolate your logic for each day into its own specific function, then build a map that has days as keys and the appropriate method as values. With a combination of putting your pickup dates into their own methods as I've said above, and then using lamdas to do your dropoff logic, I think the code becomes significantly cleaner. — user124493
– user124493, Commented Nov 25, 2009 at 15:56

BenMorel · Accepted Answer · 2014-01-12 17:47:07Z

You definitely should refactor the dropoffDate out - because the code is duplicated 3 times! The simplest cleanup: I would introduce a function to check the pickupDate and another to check the dropoffDate:

private bool IsPickupWeekend(DateTime pickupDate)
{
    switch (pickupDate.DayOfWeek)
            {
                case DayOfWeek.Thursday:
                    return pickupDate.Hour >= 12;
                case DayOfWeek.Friday:                    
                case DayOfWeek.Saturday:
                    return true;
            }
        }
        return false;
}

private bool IsWeekendDropOff(DateTime dropoffDate)
{
    switch (dropoffDate.DayOfWeek)
                    {
                        case DayOfWeek.Sunday:
                            return true;
                        case DayOfWeek.Monday:
                            if (dropoffDate.Hour <= 12)
                            {
                                return true;
                            }
                            return false;
                    }
                    return false;

}

And now your main function is a 2 liner:

if (ts.TotalDays >= 2 && ts.TotalDays <= 4)
{
    return IsPickupWeekend(pickupDate) && IsWeekendDropOff(dropoffDate);
}

@Grzenio: Should not IsWeekendDropOff function have Saturday as the day for Drop OFf? Considering PickUpDate is 1300 Hr on Thursday then Drop Date can be Saturday 1400 Hr.
See msdn.microsoft.com/en-us/library/system.datetime.hour.aspx specifically "The hour component, expressed as a value between 0 and 23"
12 PM as mentioned in question is 12 noon i.e in the clock of 0-23 it is it is 12 so should not Saturday be considered a day for DropOff? en.wikipedia.org/wiki/12-hour_clock
I have just copied the bits of code from the question - the author asked only to provide a "less ugly" version - so I assume the code in the question works "correctly".

bruno conde · Accepted Answer · 2009-11-25 15:31:43Z

3

I think you could extract a method here:

private bool ValidateDropoff(DateTime dropoffDate)
{
    switch (dropoffDate.DayOfWeek)
    {
        case DayOfWeek.Sunday:
           return true;
        case DayOfWeek.Monday:
           return dropoffDate.Hour <= 12;
        default:
           return false;
    }
}

answered Nov 25, 2009 at 15:31

bruno conde

48.3k15 gold badges103 silver badges118 bronze badges

3 Comments

Gorpik Over a year ago

You need a 'return false;' at the end; otherwise, not every path will return a value. Alternatively, just move the 'return false;' you have outside the switch.

bruno conde Over a year ago

@Gorpik, the default case takes care of this. Every path will in fact return a value.

Ashish Jain Over a year ago

Should not Saturday be a case in this code? Please let me know will this code work in case PickUpDate: Thursday 1300 Hr and Drop Date: Saturday 1400 Hr?

ChaosPandion · Accepted Answer · 2009-11-25 15:33:49Z

if (ts.TotalDays >= 2 && ts.TotalDays <= 4)
{
    var hour_limit = new Func<Boolean>(() => {
        switch (dropoffDate.DayOfWeek)
        {
            case DayOfWeek.Sunday:
                return true;
            case DayOfWeek.Monday:
                return dropoffDate.Hour <= 12;
            default:
                return false;
        }

    });

    switch (pickupDate.DayOfWeek)
    {
        case DayOfWeek.Thursday:
            if (pickupDate.Hour >= 12)  return hour_limit();
            break;
        case DayOfWeek.Friday:
        case DayOfWeek.Saturday:
            return hour_limit();
        default: 
            break;
    }
}

return false;

Mick Walker · Accepted Answer · 2009-11-25 15:34:45Z

Not much clearer, but here you go:

public bool ValidateWeekend(DateTime pickupDate, DateTime dropoffDate){
    TimeSpan ts = dropoffDate.Subtract(pickupDate);

    if (ts.TotalDays >= 2 && ts.TotalDays <= 4){
        switch (pickupDate.DayOfWeek){
            case DayOfWeek.Thursday:
                if (pickupDate.Hour >= 12){
                    switch (dropoffDate.DayOfWeek){
                        case DayOfWeek.Sunday:
                            return true;
                        case DayOfWeek.Monday:
                            return dropoffDate.Hour <= 12;
                    }
                }
                break;
            case DayOfWeek.Friday:
                switch (dropoffDate.DayOfWeek){
                    case DayOfWeek.Sunday:
                        return true;
                    case DayOfWeek.Monday:
                        return dropoffDate.Hour <= 12;
                }
                break;
            case DayOfWeek.Saturday:
                switch (dropoffDate.DayOfWeek){
                    case DayOfWeek.Sunday:
                        return true;
                    case DayOfWeek.Monday:
                        return dropoffDate.Hour <= 12;
                }
                return false;
        }
    }
    return false;
}

Pat · Accepted Answer · 2009-11-25 15:36:03Z

My first crack:

if (ts.TotalDays >= 2 && ts.TotalDays <= 4)
        {
            switch (pickupDate.DayOfWeek)
            {
                case DayOfWeek.Thursday:
                case DayOfWeek.Friday:
                case DayOfWeek.Saturday:
                    if (pickupDate.DayOfWeek == DayOfWeek.Thursday && pickupDate.Hour <= 12)
                        return false;

                    switch (dropoffDate.DayOfWeek)
                    {
                        case DayOfWeek.Sunday:
                            return true;
                        case DayOfWeek.Monday:
                            return dropoffDate.Hour <= 12;
                    }
                    return false;

                default:
                    return false;
            }
        }
        return false;

harryovers · Accepted Answer · 2009-11-25 15:36:11Z

0

in the switch try

retrun (dropoffDate.DayOfWeek == DayOfWeek.Sunday && dropoffDate.Hour <= 12 || dropoffDate.DayOfWeek == DayOfWeek.Sunday)

answered Nov 25, 2009 at 15:36

harryovers

3,1482 gold badges37 silver badges57 bronze badges

Comments

Sean Barlow · Accepted Answer · 2009-11-25 15:41:46Z

I would do it something like this

public bool ValidateWeekend(DateTime pickupDate, DateTime dropoffDate)
{
    TimeSpan ts = dropoffDate.Subtract(pickupDate);

    if (ts.TotalDays >= 2 && ts.TotalDays <= 4)
    {
        switch (pickupDate.DayOfWeek)
        {
            case DayOfWeek.Thursday:
                if (pickupDate.Hour >= 12)
                {
                    reurn DayOfWeek(dropOffDate.DayOfWeek);
                }
                break;
            case DayOfWeek.Friday, DayOfWeek.Saturday:
                {
                    return DayOfWeek(dropOffDate.DayOfWeek);
                }
        }
    }
    return false;
}

public bool DayOfWeek(DateTime dropOffDate)
    {
switch (dropoffDate.DayOfWeek)
    {
        case DayOfWeek.Sunday:
            {
                return true;
            }
        case DayOfWeek.Monday:
            {
                if (dropoffDate.Hour <= 12)
                    {
                        return true;
                    }
                return false;
            }
       return false;
   }
 }

Chuck Conway · Accepted Answer · 2009-11-25 15:49:39Z

Here is my stab at it:

  /// <summary>
    /// Gets the weekend days.
    /// </summary>
    /// <returns></returns>
    public List<DayOfWeek> GetWeekendDays()
    {
        List<DayOfWeek> days = new List<DayOfWeek>()
                                   {
                                       DayOfWeek.Thursday,
                                       DayOfWeek.Friday,
                                       DayOfWeek.Sunday
                                   };
        return days;
    }

    /// <summary>
    /// Validates the weekend.
    /// </summary>
    /// <param name="pickupDate">The pickup date.</param>
    /// <param name="dropoffDate">The dropoff date.</param>
    /// <returns></returns>
    public bool ValidateWeekend(DateTime pickupDate, DateTime dropoffDate)
    {
        bool isValid = false;
        TimeSpan ts = dropoffDate.Subtract(pickupDate);

        if (ts.TotalDays >= 2 && ts.TotalDays <= 4)
        {
            List<DayOfWeek> days = GetWeekendDays();

            foreach (DayOfWeek day in days)
            {
                if(pickupDate.DayOfWeek == day)
                {
                   isValid = ValidateDropOff(dropoffDate);
                    break;
                }
            }
        }

        return isValid;
    }

    /// <summary>
    /// Validates the drop off.
    /// </summary>
    /// <param name="dropoffDate">The dropoff date.</param>
    /// <returns></returns>
    private static bool ValidateDropOff(DateTime dropoffDate)
    {
        bool isValidDropOff = (dropoffDate.DayOfWeek == DayOfWeek.Sunday);

        if(dropoffDate.DayOfWeek == DayOfWeek.Monday)
        {
            if (dropoffDate.Hour <= 12)
            {
                isValidDropOff = true;
            }
        }

        return isValidDropOff;
    }

Andy Davis · Accepted Answer · 2013-03-11 11:52:01Z

    private readonly TimeSpan Midday = new TimeSpan(12, 0, 0);

    public bool ValidateWeekend(DateTime pickupDate, DateTime dropoffDate)
    {
        TimeSpan lengthOfTrip = dropoffDate.Subtract(pickupDate);

        if (lengthOfTrip.TotalDays < 2 || lengthOfTrip.TotalDays > 4)
            return false;

        return IsPickupDateConsideredWeekend(pickupDate) && IsDropoffDateConsideredWeekend(dropoffDate);
    }

    private bool IsPickupDateConsideredWeekend(DateTime pickupdate)
    {
        if (pickupdate.DayOfWeek == DayOfWeek.Thursday && pickupdate.TimeOfDay > Midday)
            return true;
        return false;
    }

    private bool IsDropoffDateConsideredWeekend(DateTime dropoffDate)
    {
        if (dropoffDate.DayOfWeek == DayOfWeek.Monday && dropoffDate.TimeOfDay <= Midday)
            return true;
        return false;
    }

Collectives™ on Stack Overflow

C# Switch Statement refactoring

9 Answers 9

5 Comments

3 Comments

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Hot Network Questions

Collectives™ on Stack Overflow

9 Answers 9

5 Comments

3 Comments

Comments

Comments

Comments

Comments

Comments

Comments

Comments

Related