Can anyone simplify this Algorithm for me?

Question

Basically I just want to check if one time period overlaps with another. Null end date means till infinity. Can anyone shorten this for me as its quite hard to read at times. Cheers

    public class TimePeriod
    {
        public DateTime StartDate { get; set; }
        public DateTime? EndDate { get; set; }

        public bool Overlaps(TimePeriod other)
        {
            // Means it overlaps
            if (other.StartDate == this.StartDate
                || other.EndDate == this.StartDate
                || other.StartDate == this.EndDate
                || other.EndDate == this.EndDate)
                return true;

            if(this.StartDate > other.StartDate)
            {
                // Negative
                if (this.EndDate.HasValue)
                {
                    if (this.EndDate.Value < other.StartDate)
                        return true;
                    if (other.EndDate.HasValue && this.EndDate.Value < other.EndDate.Value)
                        return true;
                }

                // Negative
                if (other.EndDate.HasValue)
                {
                    if (other.EndDate.Value > this.StartDate)
                        return true;
                    if (this.EndDate.HasValue && other.EndDate.Value > this.EndDate.Value)
                        return true;
                }
                else
                    return true;
            }
            else if(this.StartDate < other.StartDate)
            {
                // Negative
                if (this.EndDate.HasValue)
                {
                    if (this.EndDate.Value > other.StartDate)
                        return true;
                    if (other.EndDate.HasValue && this.EndDate.Value > other.EndDate.Value)
                        return true;
                }
                else
                    return true;

                // Negative
                if (other.EndDate.HasValue)
                {
                    if (other.EndDate.Value < this.StartDate)
                        return true;
                    if (this.EndDate.HasValue && other.EndDate.Value < this.EndDate.Value)
                        return true;
                }
            }

            return false;
        }
    }

Great question, but may I suggest a better name? How about "Concise way to tell if two DateTime pairs overlap?" — Joey Adams, Apr 20 '10 at 01:38
A typical case of 'mentally bruteforcing' a problem, by trying/if-casing all possible constellations. People often immediatelly try to solve problems, instead of really thinking about the problem. The solution then comes much easier and more likely it is a well thought clear one. — Philip Daubmeier, Apr 20 '10 at 01:43
Whoa!!! Beast of a code. But you only need to simplify by checking your "earlier_date.endDate" date is null (in which case later dates would overlap) then compare "earlier_date.endDate" to "later_date.Start" date for the overlap — Dark Star1, Apr 20 '10 at 01:58

score 15 · Accepted Answer · answered Apr 20 '10 at 01:24

15

public bool Overlaps(TimePeriod other)
{
    return (other.StartDate >= StartDate && 
               (EndDate == null || other.StartDate <= EndDate.Value)) ||
           (StartDate >= other.StartDate &&
               (other.EndDate == null || StartDate <= other.EndDate.Value))
}

answered Apr 20 '10 at 01:24

Adam Robinson

182,639
35
285
343

@Adam, Check your algorithm... First comparison must be startdate and enddate, not both startdates. startdate = 5 jan, enddate = 10jan; other.start = 1 Jan, other.enddate = 1 Feb will return false – Charles Bretana Apr 20 '10 at 01:44
1

@Charles: No, it won't. StartDate > other.StartDate && StartDate < other.EndDate. – Adam Robinson Apr 20 '10 at 02:03
@Duncan: Just out of curiosity, what "common code" are you referring to? I don't see any repeated expressions... – Adam Robinson Apr 20 '10 at 13:53
@Adam Robinson: `(testDate >= rangeStart && (rangeEnd == null || testDate <= rangeEnd))`. While I was being mildly facetious (because you can take refactoring too far) the solution initially sprang to my mind in SQL using BETWEEN, which is what that expression is. Maybe the method could simply test if `other` overlaps `this` and then there could be a static function taking two time periods as arguments. – Duncan Apr 20 '10 at 22:28

score 11 · Answer 2 · answered Apr 20 '10 at 01:29

11

How about this one:

public bool Overlaps(TimePeriod other)
{
    bool isOtherEarlier = this.StartDate > other.StartDate;
    TimePeriod earlier = isOtherEarlier  ? other : this;
    TimePeriod later = isOtherEarlier ? this : other;
    return !earlier.EndDate.HasValue || earlier.EndDate > later.StartDate;
}

answered Apr 20 '10 at 01:29

Philip Daubmeier

14,584
5
41
77

Not quite one line like Adams solution, but I think a little more readable. But that is maybe just a matter of taste. – Philip Daubmeier Apr 20 '10 at 01:35
This also wrong... First comparison must check this.StartDate against other.endDate – Charles Bretana Apr 20 '10 at 01:54
I think the only thing "wrong" with this is that phil chose to assume that dates which touch boundaries are not considered to be overlapping. But that's really just a matter of choice, rather than a matter of right or wrong. Simply change > to >= to go the other way. (The OP went the other way) – Igby Largeman Apr 20 '10 at 03:58

score 3 · Answer 3 · edited May 23 '17 at 12:11

Check this out: DateTimeOverlaps

Very generally, if all variables are nullable datetimes, then

   return (StartA.HasValue? StartA.Value:DateTime.Minimum) <= 
             (EndB.HasValue? EndB.Value:DateTime.Maximum)  && 
          (EndA.HasValue? EndA.Value:DateTime.Maximum) >= 
              (StartB.HasValue? StartB.Value:DateTime.Minimum);

The concept, (as explained in link) is very simple, and is simply and concisely expressed above.

If the start is before the others end, and the end is after the other start, you have overlap. This says it all, and all that is necessary, in one simple sentence with two clauses, and whatever code you write should concisely map to that simple concept without obfuscating it. Adding extra unecessary complexity does not add understanding, it only adds length.

Fail Case 1: TopStart AFTER other End - Fail

       |----------|
|--|

Fail Case 2: TopEnd AFTER other start - Fail

   |-----------|
                   |------|

In all other cases, start is before other end, and end is after other start.

case A

    |----------|  
|-----|

case B

    | ---------|
|-------------------|

case C

|-----------|
   |------|

case D

   |-----------|
       |-------------|

Don't think it handles null end date – Joshscorp Apr 20 '10 at 01:23 — Joshscorp, Apr 20 '10 at 01:23

score 2 · Answer 4 · answered Apr 20 '10 at 03:52

Any time you're dealing with pure boolean logic, you can distill your algorithm down to a single statement. But don't assume that just because you can, you should. Unless performance is vital, always go for readable code over compact code. (Not that compactness == performance, necessarily)

This is easy to read because it's comprised entirely of single AND expressions, and it's obvious that they all determine a non-overlap:

public bool Overlaps(TimePeriod other)
{
    if (other.EndDate.HasValue && other.EndDate < StartDate)
        return false;

    if (EndDate.HasValue && EndDate < other.StartDate)
        return false;

    if (!EndDate.HasValue && other.EndDate < StartDate)
        return false;

    if (!other.EndDate.HasValue && EndDate < other.StartDate)
        return false;

    return true;
}

Not that the other answers are bad (I like Adam's; his formatting is obviously designed to aid readability). I'm just saying this because it's clear you're a beginner, and I think this is one lesson that isn't heeded enough (I'm guilty). Somebody (I think Martin Fowler) once said something like: "Any fool can write code that a computer understands, but a good programmer can write code that a human understands."

Can anyone simplify this Algorithm for me?

4 Answers4