Skip to content

InOpenRange and InClosedRange have inverted semantics (endpoint behavior is swapped vs. the names) #3

Description

@PFalkowski

Summary

InOpenRange and InClosedRange have inverted endpoint semantics — each one is the mathematical definition of the other. By the standard convention an open interval excludes its endpoints (from, to) and a closed interval includes them [from, to]. The implementations are reversed:

// Extensions.Standard/Utilities.cs
public static bool InClosedRange<T>(this T value, T from, T to) where T : IComparable<T>
{
    return value.CompareTo(from) > 0 && value.CompareTo(to) < 0;   // strict ends  → this is an OPEN interval (from, to)
}

public static bool InOpenRange<T>(this T value, T from, T to) where T : IComparable<T>
{
    return value.CompareTo(from) >= 0 && value.CompareTo(to) <= 0; // inclusive ends → this is a CLOSED interval [from, to]
}

So InOpenRange returns true on the boundary and InClosedRange returns false on the boundary — the opposite of what the names promise.

Reproduction

10.InOpenRange(0, 10);    // True   — but "open" should exclude 10
0.InOpenRange(0, 10);     // True   — but "open" should exclude 0
0d.InOpenRange(0, 0);     // True

10.InClosedRange(0, 10);  // False  — but "closed" should include 10
5.InClosedRange(0, 10);   // True   (only the interior)

The existing unit tests (ExtensionsTest.InOpenRangeTest2, InClosedRangeTest2, the *Dates variants) assert exactly this inverted behavior, so the test suite passes while codifying the swap.

Impact

This is a quiet footgun: the names read as precise mathematical terms, so callers reasonably assume InOpenRange excludes the endpoints. In a downstream project a price-fill range check price.InOpenRange(low, high) was assumed to be exclusive; it is actually inclusive, which silently invalidated a bug analysis built on that assumption. Anyone reaching for "open interval" semantics gets the closed one (and vice versa) with no compiler help.

Suggested fix

A straight swap of the two bodies is the correct behavior but is binary-breaking for existing callers who have adapted to the current behavior. Options, least-disruptive first:

  1. Add correctly-named methods and deprecate the misleading ones. Introduce e.g. InRangeExclusive / InRangeInclusive (or Between / BetweenInclusive) with the obvious semantics, and mark InOpenRange / InClosedRange [Obsolete(...)] pointing at the replacements. Non-breaking.
  2. Document the inversion in XML doc comments on both methods (<summary>/<remarks>) so IntelliSense warns readers, even if the names stay.
  3. Swap the bodies and ship a major version bump, updating the tests — the clean fix, but breaking.

Happy to send a PR for whichever direction you prefer.

Environment

  • Package: Extensions.Standard 8.1.0 (current on master / NuGet)
  • Methods: Extensions.Standard/Utilities.csInClosedRange<T> / InOpenRange<T>
  • Found via the PFalkowski/XtbClient simulator

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions