Skip to content

C5: dedent test class to match file-scoped namespace convention (closes #92, #93)#133

Merged
Chris-Wolfgang merged 2 commits into
vNextfrom
stack/c5-code-review-fixes
Jun 3, 2026
Merged

C5: dedent test class to match file-scoped namespace convention (closes #92, #93)#133
Chris-Wolfgang merged 2 commits into
vNextfrom
stack/c5-code-review-fixes

Conversation

@Chris-Wolfgang
Copy link
Copy Markdown
Owner

Summary

Code-review (C5) pass on IComparable-Extensions, plus the dead-code
sweep (#93). Whitespace-only fix this PR; the rest of the audit had
no findings worth a change.

What changed

tests/Wolfgang.Extensions.IComparable.Tests.Unit/IComparableExtensionsTests.cs
— class + members dedented by 4 spaces. The file uses a file-scoped
namespace (namespace X;) but the class kept block-namespace
indentation. The fix is pure whitespace.

Audit findings

Source (src/.../IComparableExtensions.cs, 74 LOC) — clean

  • Both public methods (IsBetween<T>, IsInRange<T>) have complete
    XML docs with <summary>, <param>, <returns>, <exception>
  • Both correctly throw ArgumentNullException(nameof(value)) when
    value is null
  • Multi-line argument lists follow CLAUDE.md's Allman style (opening
    paren on its own line, each arg indented, closing paren on its own
    line)
  • File-scoped namespace
  • 3 blank lines between members
  • No async, no allocations in tight loops, no string concat, no
    LINQ chains — there's nothing tight to optimize in a CompareTo
    wrapper

Tests (190 LOC) — clean apart from indentation

  • All 16 tests assert
  • All follow the MethodName_when_condition_expected_result naming
  • Cover the boundary matrix: lower-edge, upper-edge, mid-range,
    outside-low, outside-high, equal-bounds, inverted-bounds, null-value
  • Cover four T shapes: int, DateTime, string, char
  • No private helpers, no dead test code

Dead code (#93)

  • src/ has 2 public methods, both used by tests — no dead surface
  • tests/ has 16 public [Fact]/[Theory] methods, no private
    helpers, no unused symbols
  • examples/ has two Program.Main methods (net462 + net8) — both
    the entry points, neither dead

Closing #93 as "no dead code found" via this PR's Closes line.

Possible future work (out of scope for this PR)

  • Document and test behavior when lowerBound or upperBound is
    null — current code defers to T's CompareTo contract
    (string.CompareTo(null) returns positive, so IsBetween(null, "z")
    is true). Worth a follow-up issue if you want that contract
    formalized.
  • Add a custom-IComparable<T> test (e.g. a Money record) — the
    README rewrite in D2/D-readme: rewrite the stub README into a real one (closes #118, #102) #131 shows this usage, but no test exists.
    Low priority since int/DateTime/string/char already exercise
    generic dispatch.

Stacked on vNext (PR #129).

The test file converted to file-scoped namespace ('namespace X;') but
the class body kept the old block-namespace indentation (4 spaces for
the class, 8 for members). Under file-scoped namespaces, contents
belong at column 0 with members at 4 spaces.

Pure whitespace change — no behavior or test count change.

Code-review pass otherwise found no issues:
- Public surface (IsBetween / IsInRange): full XML docs, correct
  ArgumentNullException, multi-line argument lists in CLAUDE.md
  Allman style, file-scoped namespace, 3 blank lines between members
- Tests: all assert, all named per the MethodName_when_x_y_z pattern,
  cover bounds (lower / upper / inverted / equal-bounds), cover null
  value, cover int/DateTime/string/char value types
- No dead code in src/, tests/, or examples/
Copilot AI review requested due to automatic review settings June 1, 2026 15:13
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review any files in this pull request.

@Chris-Wolfgang Chris-Wolfgang merged commit 696d3dc into vNext Jun 3, 2026
@Chris-Wolfgang Chris-Wolfgang deleted the stack/c5-code-review-fixes branch June 3, 2026 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants