0

With the enhancements in pattern matching in c# 8 have been wondering if there is a better way of writing the following:

private bool HasOverlapTime(TimeSpan requestFrom, TimeSpan requestTo, TimeSpan detailFrom, TimeSpan detailTo)
{
    var timesOverlap = (requestFrom >= detailFrom && requestFrom < detailTo); //time From falls into range

    timesOverlap = !timesOverlap ? requestTo <= detailTo && requestTo > detailFrom : timesOverlap; // time to falls in range

    timesOverlap = !timesOverlap ? requestFrom <= detailFrom && requestTo >= detailTo : timesOverlap; // previous row false into new range

    return timesOverlap;
}
Panagiotis Kanavos
  • 120,703
  • 13
  • 188
  • 236
Yunus
  • 13
  • 7
  • See [this answer](https://stackoverflow.com/a/13513973/1086121). There's nothing in C# 8's pattern matching for `<` or `>` – canton7 Mar 04 '20 at 10:57
  • 1
    Did you intend to have `TimeSpan`s here instead of `DateTime`s? It doesn't seem to make any sense to use `TimeSpan`s for this. – JLRishe Mar 04 '20 at 14:25
  • Once you calculate the truth table for `Overlaps`, perhaps try drawing some diagrams, you'll see it's as simple as Matthew Watson's solution. – Panagiotis Kanavos Mar 04 '20 at 14:39
  • Thanks, seeing Matthew's solution suddenly it makes sense! My saving grace is I didn't write the method.. was looking at how I could make it work for a new requirement and possibly simplify in the process – Yunus Mar 04 '20 at 14:54
  • Have changed it to use DateTime and there was already a method in the project which another extension method which another dev had implemented for this scenario. – Yunus Mar 04 '20 at 14:56

3 Answers3

3

Assuming that HasOverlapTime() should return true if the two time ranges overlap, and also that the time ranges represent "half-open" intervals (i.e. the end of the range is NOT included in the range), then a much simpler implementation is:

private static bool HasOverlapTime2(TimeSpan requestFrom, TimeSpan requestTo, TimeSpan detailFrom, TimeSpan detailTo)
{
    return (detailTo > requestFrom) && (detailFrom < requestTo);
}

No need to use pattern matching to simplify the code.

Note that your code is using half-open intervals. If instead you want to use closed intervals (i.e., the interval includes its end time), then you would change the test to:

private static bool HasOverlapTime(TimeSpan requestFrom, TimeSpan requestTo, TimeSpan detailFrom, TimeSpan detailTo)
{
    return (detailTo >= requestFrom) && (detailFrom <= requestTo);
}
Matthew Watson
  • 104,400
  • 10
  • 158
  • 276
  • @JLRishe once you calculate the truth table for `Overlaps` you'll see that it really is that simple. – Panagiotis Kanavos Mar 04 '20 at 14:36
  • @JLRishe if you search for `period overlaps` in general, you'll find similar SO questions [like this one](https://stackoverflow.com/questions/17106670/how-to-check-a-timeperiod-is-overlapping-another-time-period-in-java) – Panagiotis Kanavos Mar 04 '20 at 14:37
  • 1
    @JLRishe The condition is correct. You can see a visualization in the [overlap tag info](https://stackoverflow.com/tags/overlap/info). – Zohar Peled Mar 04 '20 at 15:04
0

A suggestion (more than an answer) is to refactor your code using logical operators. It'll reduce code size and over complexification.

&= and |= operators could fit well in your case.

another point - this time using C# 8 - that could fit as well is the deconstruct pattern. it allows you to extract those data inside a Tuple from the class, and then write your logic inside a C#8 "property pattern match" switch.

  • I have broke it down to multiple lines for now just to make it easier to get my head round the scenarios, was one statement using ||. Will take a look at deconstruct, have seen some examples of it – Yunus Mar 04 '20 at 12:07
  • Alright, but keep in mind that Matthew Watson's solution is actually the way to go in order to reduce complexity and increase lisibility :). Using deconstruct should only makes your structure better, not your algoritm ! – Julien Boyer Mar 04 '20 at 12:21
0

Used this method in the end which was already in the solution

        var overlappingRanges = ranges.Any(a =>
            ranges.Any(b => b != a && !(a.DateFrom > b.DateTo || b.DateFrom > a.DateTo)));
        if (!noSameDay)
        {
            return overlappingRanges;
        }
        return overlappingRanges || ranges.Any(c => c.DateTo == c.DateFrom);
Yunus
  • 13
  • 7
  • That's a different sort of beast (problem). The expression is trivial, but trying to find overlapping ranges in a set is not. In this case you're performing N^2 comparisons. – Panagiotis Kanavos Mar 04 '20 at 15:12
  • I'm sure there are efficient algorithms for this, but perhaps one way you could accelerate this would be to scan the ranges just once and "mark" individual dates in each range as covered, in a bitfield, array or perhaps a HashSet. Once you found a date that was already covered, you'd know the full set of ranges contains overlaps – Panagiotis Kanavos Mar 04 '20 at 15:16