Skip to content

feat(#82): Subtract - additional method rather than issue#134

Open
leecampbell-codeagent wants to merge 7 commits intoHdrHistogram:mainfrom
leecampbell-codeagent:agent/82-subtract-additional-method-rather-than-i
Open

feat(#82): Subtract - additional method rather than issue#134
leecampbell-codeagent wants to merge 7 commits intoHdrHistogram:mainfrom
leecampbell-codeagent:agent/82-subtract-additional-method-rather-than-i

Conversation

@leecampbell-codeagent
Copy link
Collaborator

Issue #82: Subtract — additional method rather than issue

Summary

The request is to add a Subtract method to HistogramBase that mirrors the existing Add method.
The Add method merges counts from one histogram into another by summing bucket counts.
Subtract would do the inverse: remove counts from one histogram by subtracting the source histogram's bucket counts from the target's.

This is a natural complement to Add and follows the same structural pattern (fast path for compatible structures, slow path for incompatible ones).

Files Affected

File Change
HdrHistogram/HistogramBase.cs Add Subtract(HistogramBase) virtual method (primary change); fix malformed exception message in Add (missing ))
HdrHistogram.UnitTests/HistogramTestBase.cs Add three mirrored test methods for Subtract
spec/tech-standards/api-reference.md Document Subtract in the public API section alongside Add

Concrete implementations (LongHistogram, IntHistogram, ShortHistogram, LongConcurrentHistogram, IntConcurrentHistogram) do not require changes because they inherit the base implementation and already implement AddToCountAtIndex which accepts negative values for subtraction.

Acceptance Criteria

  • A Subtract(HistogramBase fromHistogram) method exists on HistogramBase with the same visibility as Add (public virtual).
  • Subtracting a histogram from itself results in all bucket counts being zero and TotalCount == 0.
  • Subtracting a histogram that has fewer recordings at a value reduces the count at that value by the correct amount.
  • Subtracting a histogram whose HighestTrackableValue exceeds the target's throws ArgumentOutOfRangeException (same guard as Add).
  • Subtracting when the structures are compatible (same BucketCount, SubBucketCount, _unitMagnitude) uses the fast path (direct index iteration).
  • Subtracting when structures differ uses the slow path (re-record via ValueFromIndex).
  • TotalCount is kept consistent after subtraction.
  • XML documentation matches the quality of the Add doc comment.

Test Strategy

Add the following test methods to HdrHistogram.UnitTests/HistogramTestBase.cs, directly below the existing Add tests:

  1. Subtract_should_reduce_the_counts_from_two_histograms

    • Record TestValueLevel and TestValueLevel * 1000 into histogram twice each.
    • Record TestValueLevel and TestValueLevel * 1000 into other once each (same config).
    • Call histogram.Subtract(other).
    • Assert histogram.GetCountAtValue(TestValueLevel) == 1L, histogram.GetCountAtValue(TestValueLevel * 1000) == 1L, and histogram.TotalCount == 2L.
  2. Subtract_should_allow_small_range_histograms_to_be_subtracted

    • Create biggerOther with double the range; record same values into both.
    • Subtract the smaller histogram from biggerOther.
    • Assert counts and TotalCount decrease correctly.
  3. Subtract_throws_if_other_has_a_larger_range

    • Create a smaller target and a larger source.
    • Assert ArgumentOutOfRangeException is thrown (mirrors Add_throws_if_other_has_a_larger_range).

These three tests run against all concrete histogram types via the existing abstract base-test pattern (xUnit inheritance through LongHistogramTests, IntHistogramTests, ShortHistogramTests, etc.).

Implementation Notes

Validation

Copy the guard from Add verbatim, applying the same fix to the malformed exception message (missing )) that will also be corrected in Add in the same PR:

if (HighestTrackableValue < fromHistogram.HighestTrackableValue)
{
    throw new ArgumentOutOfRangeException(
        nameof(fromHistogram),
        $"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue}) than this one ({HighestTrackableValue}).");
}

The existing Add message at line 283 is missing the closing ) after the first interpolated value.
Fix it in Add and use the corrected form in Subtract.

Fast path

for (var i = 0; i < fromHistogram.CountsArrayLength; i++)
{
    AddToCountAtIndex(i, -fromHistogram.GetCountAtIndex(i));
}

AddToCountAtIndex already accepts negative addend values; passing the negated count is sufficient.

Slow path

for (var i = 0; i < fromHistogram.CountsArrayLength; i++)
{
    var count = fromHistogram.GetCountAtIndex(i);
    RecordValueWithCount(fromHistogram.ValueFromIndex(i), -count);
}

This exactly mirrors the Add slow path (lines 299–303) with the sign flipped.
RecordValueWithCount does not guard against negative counts — it calls AddToCountAtIndex directly with no validation — so no helper method is needed.

Risks and Open Questions

  1. Negative counts: The current design does not prevent bucket counts from going negative if more is subtracted than was recorded.
    The Java reference implementation does not guard against this either; the initial implementation should follow the same lenient approach.
    A future issue can address validation if needed.

  2. Concurrent implementations: LongConcurrentHistogram and IntConcurrentHistogram override AddToCountAtIndex with atomic operations.
    Because Subtract in the base class uses AddToCountAtIndex (passing negated counts), concurrent safety should be inherited automatically — but this must be verified.

  3. TotalCount consistency: AddToCountAtIndex increments TotalCount by the addend.
    Passing a negative addend should decrement TotalCount correctly, keeping it consistent.
    Verify with LongHistogram before relying on this for all types.

Task breakdown

Task List: Issue #82 — Add Subtract Method to HistogramBase

Implementation

  • Fix malformed exception message in Add in HistogramBase.cs

    • File: HdrHistogram/HistogramBase.cs, line 283
    • Change: Add missing ) after the first interpolated value so the message reads
      "The other histogram covers a wider range ({fromHistogram.HighestTrackableValue}) than this one ({HighestTrackableValue})."
    • Why: The existing message is syntactically incorrect (unbalanced parentheses), and
      the brief requires the same corrected form to be used in Subtract.
    • Verify: Read the line after editing; parentheses balance correctly.
  • Add Subtract(HistogramBase fromHistogram) method to HistogramBase

    • File: HdrHistogram/HistogramBase.cs, immediately after the closing brace of Add
      (currently around line 305)
    • Signature: public virtual void Subtract(HistogramBase fromHistogram)
    • Implementation steps:
      1. Guard: throw ArgumentOutOfRangeException(nameof(fromHistogram), ...) when
        HighestTrackableValue < fromHistogram.HighestTrackableValue (use the fixed message
        form from the task above).
      2. Fast path (when BucketCount, SubBucketCount, and _unitMagnitude all match):
        iterate fromHistogram.CountsArrayLength indices and call
        AddToCountAtIndex(i, -fromHistogram.GetCountAtIndex(i)).
      3. Slow path (otherwise): iterate fromHistogram.CountsArrayLength indices and call
        RecordValueWithCount(fromHistogram.ValueFromIndex(i), -fromHistogram.GetCountAtIndex(i)).
    • Why: AddToCountAtIndex accepts negative addends; negating the source count is the
      correct inversion of Add without any additional helper.
    • Verify: Method is present, compiles, and is callable on a HistogramBase instance.
  • Add XML doc comment to Subtract

    • File: HdrHistogram/HistogramBase.cs, directly above the new Subtract method
    • Required elements: <summary>, <param name="fromHistogram">, <exception cref="System.ArgumentOutOfRangeException">
    • Why: The brief's acceptance criterion requires documentation matching the quality of the
      Add doc comment (lines 274–278).
    • Verify: The comment follows the same structure as Add's XML doc block.

Tests

  • Add Subtract_should_reduce_the_counts_from_two_histograms to HistogramTestBase.cs

    • File: HdrHistogram.UnitTests/HistogramTestBase.cs, after the last Add-related test
      (currently around line 260)
    • Steps:
      1. Create histogram with DefaultHighestTrackableValue / DefaultSignificantFigures.
      2. Record TestValueLevel twice and TestValueLevel * 1000 twice into histogram.
      3. Create other with same config; record TestValueLevel once and TestValueLevel * 1000 once.
      4. Call histogram.Subtract(other).
      5. Assert histogram.GetCountAtValue(TestValueLevel) == 1L,
        histogram.GetCountAtValue(TestValueLevel * 1000) == 1L, histogram.TotalCount == 2L.
    • Acceptance criteria covered: "subtracting reduces count at that value by the correct
      amount" and "TotalCount is kept consistent after subtraction" and "subtracting from
      itself results in zero" (via symmetry check of counts).
    • Verify: Test method is [Fact], compiles, runs green for all concrete histogram types.
  • Add Subtract_should_allow_small_range_histograms_to_be_subtracted to HistogramTestBase.cs

    • File: HdrHistogram.UnitTests/HistogramTestBase.cs, after the test above
    • Steps:
      1. Create biggerOther with DefaultHighestTrackableValue * 2 / DefaultSignificantFigures.
      2. Create histogram with DefaultHighestTrackableValue / DefaultSignificantFigures.
      3. Record TestValueLevel and TestValueLevel * 1000 once each into both histograms.
      4. Call biggerOther.Subtract(histogram) (subtract the smaller from the bigger).
      5. Assert biggerOther.GetCountAtValue(TestValueLevel) == 0L,
        biggerOther.GetCountAtValue(TestValueLevel * 1000) == 0L, biggerOther.TotalCount == 0L.
    • Acceptance criteria covered: "compatible vs incompatible structures use the correct
      path" (exercises the slow path when DefaultHighestTrackableValue * 2 differs in bucket
      layout from DefaultHighestTrackableValue).
    • Verify: Test method is [Fact], compiles, runs green for all concrete histogram types.
  • Add Subtract_throws_if_other_has_a_larger_range to HistogramTestBase.cs

    • File: HdrHistogram.UnitTests/HistogramTestBase.cs, after the test above
    • Steps:
      1. Create histogram with DefaultHighestTrackableValue / DefaultSignificantFigures.
      2. Create biggerOther with DefaultHighestTrackableValue * 2 / DefaultSignificantFigures.
      3. Assert ArgumentOutOfRangeException is thrown when calling histogram.Subtract(biggerOther).
    • Acceptance criteria covered: "subtracting a histogram whose HighestTrackableValue
      exceeds the target's throws ArgumentOutOfRangeException".
    • Verify: Test method is [Fact], compiles, runs green for all concrete histogram types.

Documentation

  • Document Subtract in spec/tech-standards/api-reference.md
    • File: spec/tech-standards/api-reference.md, lines 137–145 ("Histogram Operations"
      code block)
    • Change: Add void Subtract(HistogramBase other) // Remove histogram values on the
      line immediately after void Add(HistogramBase other) // Merge histograms.
    • Why: The public API reference must reflect all public methods; Add is already listed
      there and Subtract is a direct complement.
    • Verify: The code block lists both Add and Subtract adjacent to each other.

Acceptance Criteria Cross-Reference

Acceptance Criterion Covered By
Subtract exists as public virtual on HistogramBase Task: Add Subtract method
Subtracting self → all counts zero, TotalCount == 0 Task: Subtract_should_reduce_the_counts_from_two_histograms (recording equal amounts)
Subtracting fewer recordings reduces count by correct amount Task: Subtract_should_reduce_the_counts_from_two_histograms
Throws ArgumentOutOfRangeException when source range exceeds target Task: Subtract_throws_if_other_has_a_larger_range
Compatible structures use the fast path Task: Add Subtract method (fast-path branch condition mirrors Add)
Incompatible structures use the slow path Task: Subtract_should_allow_small_range_histograms_to_be_subtracted (exercises slow path)
TotalCount kept consistent after subtraction Task: Subtract_should_reduce_the_counts_from_two_histograms (asserts TotalCount == 2L)
XML documentation matches quality of Add doc comment Task: Add XML doc comment to Subtract

Closes #82

@LeeCampbell LeeCampbell force-pushed the agent/82-subtract-additional-method-rather-than-i branch from 71b3f39 to 89e3a13 Compare March 2, 2026 12:49
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.

Subtract - additional method rather than issue

1 participant