From e2d30bcf03c80548385be71b3a541760e318cbf9 Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 2 Mar 2026 00:35:25 +0000 Subject: [PATCH 1/7] plan(#82): initial brief from issue --- plan/planning/brief.md | 111 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) create mode 100644 plan/planning/brief.md diff --git a/plan/planning/brief.md b/plan/planning/brief.md new file mode 100644 index 0000000..1d415a1 --- /dev/null +++ b/plan/planning/brief.md @@ -0,0 +1,111 @@ +# 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) | +| `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 values into `histogram` and `other` (same config). + - Record one extra value into `histogram` at the same slots. + - Subtract `other` from `histogram`. + - Assert that counts equal the surplus, and `TotalCount` is correct. + +2. **`Subtract_should_allow_small_range_histograms_to_be_subtracted`** + - Create `biggerOther` with double the range; record same values into both. + - Subtract 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 + +### Fast path + +```csharp +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 + +```csharp +for (var i = 0; i < fromHistogram.CountsArrayLength; i++) +{ + var count = fromHistogram.GetCountAtIndex(i); + if (count != 0) + { + SubtractFromCountAtValue(fromHistogram.ValueFromIndex(i), count); + } +} +``` + +The slow path requires a helper `SubtractFromCountAtValue` (analogous to `RecordValueWithCount` but decrements). +Alternatively — and more simply — reuse `RecordValueWithCount` with a **negated** count if the internal implementation tolerates it (it calls `AddToCountAtIndex` which does). +Confirm this before coding; if `RecordValueWithCount` has a guard rejecting negative counts, a dedicated helper is needed. + +### Validation + +Identical guard to `Add`: + +```csharp +if (HighestTrackableValue < fromHistogram.HighestTrackableValue) +{ + throw new ArgumentOutOfRangeException( + nameof(fromHistogram), + $"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue}) " + + $"than this one ({HighestTrackableValue})."); +} +``` + +## 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. **`RecordValueWithCount` with negative count**: Check whether `RecordValueWithCount` internally rejects negative values. + If it does, the slow path must call `AddToCountAtIndex` directly (after computing the index) rather than going via `RecordValueWithCount`. + +3. **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. + +4. **`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. From d49a90ce68dd8754e212645010725671be3745fa Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 2 Mar 2026 00:39:15 +0000 Subject: [PATCH 2/7] plan(#82): review brief --- plan/planning/brief-review.md | 118 ++++++++++++++++++++++++++++++++++ 1 file changed, 118 insertions(+) create mode 100644 plan/planning/brief-review.md diff --git a/plan/planning/brief-review.md b/plan/planning/brief-review.md new file mode 100644 index 0000000..bc58b47 --- /dev/null +++ b/plan/planning/brief-review.md @@ -0,0 +1,118 @@ +# Brief Review: Issue #82 — Subtract Method + +## Verdict: Revise before implementation + +The brief is clear, well-scoped, and architecturally sound. +All three files exist and the proposed changes are feasible. +However, five specific issues need resolving before handoff to avoid implementer confusion. + +--- + +## Issue 1 — Open Question #2 is already answered: resolve it + +The brief lists as an open question whether `RecordValueWithCount` guards against negative counts. +Codebase inspection confirms it does **not**: + +```csharp +// HistogramBase.cs ~line 237 +public void RecordValueWithCount(long value, long count) +{ + var bucketIndex = GetBucketIndex(value); + var subBucketIndex = GetSubBucketIndex(value, bucketIndex); + var countsIndex = CountsArrayIndex(bucketIndex, subBucketIndex); + AddToCountAtIndex(countsIndex, count); // no validation +} +``` + +**Action:** Remove Open Question #2. +In the slow-path Implementation Note, commit to the simpler approach: +pass `-count` directly to `RecordValueWithCount`. +Delete the mention of a `SubtractFromCountAtValue` helper — it is not needed. + +--- + +## Issue 2 — Slow-path code snippet is ambiguous (two options, no decision) + +The slow-path note presents both a `SubtractFromCountAtValue` helper and `RecordValueWithCount(-count)` as alternatives without choosing one. +Issue 1 above resolves the blocker; the simpler option is now unambiguously correct. + +**Action:** Replace both slow-path snippets with a single definitive version: + +```csharp +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, +and requires no new helper. + +--- + +## Issue 3 — Slow-path has a `if (count != 0)` guard that `Add` does not + +The brief's proposed slow-path wraps the operation in `if (count != 0)`. +The actual `Add` slow path (lines 299–303) does **not** filter zero counts. + +Passing `count = 0` to `RecordValueWithCount` is harmless (adds 0, increments `TotalCount` by 0), +so the guard is unnecessary. +Keeping it creates an unexplained divergence from `Add`. + +**Action:** Drop the `if (count != 0)` guard from the slow-path snippet to match `Add` exactly. +If there is a performance reason to keep it, document it explicitly. + +--- + +## Issue 4 — Test 1 assertions are under-specified + +The brief says "Assert that counts equal the surplus, and `TotalCount` is correct" without +giving exact values. +All three `Add` tests use concrete `Assert.Equal(…)` calls. +The test author needs the same level of specificity to write a correct test without guessing. + +**Action:** Specify the setup and assertions concretely, matching the `Add` test style. +Suggested wording: + +> Record `TestValueLevel` and `TestValueLevel * 1000` into `histogram` twice each. +> Record `TestValueLevel` and `TestValueLevel * 1000` into `other` once each. +> Call `histogram.Subtract(other)`. +> Assert `histogram.GetCountAtValue(TestValueLevel) == 1L`, +> `histogram.GetCountAtValue(TestValueLevel * 1000) == 1L`, +> and `histogram.TotalCount == 2L`. + +--- + +## Issue 5 — Exception message in the brief differs from the one in `Add` + +The existing `Add` validation message (line 283) has a minor formatting issue — +the first interpolated value is not closed with `)`: + +```csharp +// existing Add — note missing closing ')' after HighestTrackableValue +$"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue} than this one ({HighestTrackableValue})." +``` + +The brief's proposed message adds the missing `)`, which is technically correct but +inconsistent with `Add`. + +**Action:** Decide explicitly: either copy the existing message verbatim (warts and all, for +consistency with `Add`) or fix both `Add` and `Subtract` in the same PR and add a note to +the Files Affected table. +Do not silently diverge. + +--- + +## Summary of required changes to the brief + +| # | Change | +|---|--------| +| 1 | Remove Open Question #2; state definitively that `RecordValueWithCount` accepts negative counts | +| 2 | Replace two slow-path options with one: `RecordValueWithCount(fromHistogram.ValueFromIndex(i), -count)` | +| 3 | Drop `if (count != 0)` guard from slow-path to match `Add` | +| 4 | Specify Test 1 setup and assertions with exact values and `Assert.Equal` calls | +| 5 | Decide whether to copy the existing `Add` exception message or fix it in both methods; document the choice | + +None of these change the scope or architecture. +After addressing them the brief is ready to move to `plan/ready/`. From 43e129d1a55a3d837536f8c6286c0bdfc62816e9 Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 2 Mar 2026 00:41:04 +0000 Subject: [PATCH 3/7] plan(#82): apply brief review feedback --- plan/planning/brief-review.md | 118 ---------------------------------- plan/planning/brief.md | 59 ++++++++--------- 2 files changed, 27 insertions(+), 150 deletions(-) delete mode 100644 plan/planning/brief-review.md diff --git a/plan/planning/brief-review.md b/plan/planning/brief-review.md deleted file mode 100644 index bc58b47..0000000 --- a/plan/planning/brief-review.md +++ /dev/null @@ -1,118 +0,0 @@ -# Brief Review: Issue #82 — Subtract Method - -## Verdict: Revise before implementation - -The brief is clear, well-scoped, and architecturally sound. -All three files exist and the proposed changes are feasible. -However, five specific issues need resolving before handoff to avoid implementer confusion. - ---- - -## Issue 1 — Open Question #2 is already answered: resolve it - -The brief lists as an open question whether `RecordValueWithCount` guards against negative counts. -Codebase inspection confirms it does **not**: - -```csharp -// HistogramBase.cs ~line 237 -public void RecordValueWithCount(long value, long count) -{ - var bucketIndex = GetBucketIndex(value); - var subBucketIndex = GetSubBucketIndex(value, bucketIndex); - var countsIndex = CountsArrayIndex(bucketIndex, subBucketIndex); - AddToCountAtIndex(countsIndex, count); // no validation -} -``` - -**Action:** Remove Open Question #2. -In the slow-path Implementation Note, commit to the simpler approach: -pass `-count` directly to `RecordValueWithCount`. -Delete the mention of a `SubtractFromCountAtValue` helper — it is not needed. - ---- - -## Issue 2 — Slow-path code snippet is ambiguous (two options, no decision) - -The slow-path note presents both a `SubtractFromCountAtValue` helper and `RecordValueWithCount(-count)` as alternatives without choosing one. -Issue 1 above resolves the blocker; the simpler option is now unambiguously correct. - -**Action:** Replace both slow-path snippets with a single definitive version: - -```csharp -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, -and requires no new helper. - ---- - -## Issue 3 — Slow-path has a `if (count != 0)` guard that `Add` does not - -The brief's proposed slow-path wraps the operation in `if (count != 0)`. -The actual `Add` slow path (lines 299–303) does **not** filter zero counts. - -Passing `count = 0` to `RecordValueWithCount` is harmless (adds 0, increments `TotalCount` by 0), -so the guard is unnecessary. -Keeping it creates an unexplained divergence from `Add`. - -**Action:** Drop the `if (count != 0)` guard from the slow-path snippet to match `Add` exactly. -If there is a performance reason to keep it, document it explicitly. - ---- - -## Issue 4 — Test 1 assertions are under-specified - -The brief says "Assert that counts equal the surplus, and `TotalCount` is correct" without -giving exact values. -All three `Add` tests use concrete `Assert.Equal(…)` calls. -The test author needs the same level of specificity to write a correct test without guessing. - -**Action:** Specify the setup and assertions concretely, matching the `Add` test style. -Suggested wording: - -> Record `TestValueLevel` and `TestValueLevel * 1000` into `histogram` twice each. -> Record `TestValueLevel` and `TestValueLevel * 1000` into `other` once each. -> Call `histogram.Subtract(other)`. -> Assert `histogram.GetCountAtValue(TestValueLevel) == 1L`, -> `histogram.GetCountAtValue(TestValueLevel * 1000) == 1L`, -> and `histogram.TotalCount == 2L`. - ---- - -## Issue 5 — Exception message in the brief differs from the one in `Add` - -The existing `Add` validation message (line 283) has a minor formatting issue — -the first interpolated value is not closed with `)`: - -```csharp -// existing Add — note missing closing ')' after HighestTrackableValue -$"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue} than this one ({HighestTrackableValue})." -``` - -The brief's proposed message adds the missing `)`, which is technically correct but -inconsistent with `Add`. - -**Action:** Decide explicitly: either copy the existing message verbatim (warts and all, for -consistency with `Add`) or fix both `Add` and `Subtract` in the same PR and add a note to -the Files Affected table. -Do not silently diverge. - ---- - -## Summary of required changes to the brief - -| # | Change | -|---|--------| -| 1 | Remove Open Question #2; state definitively that `RecordValueWithCount` accepts negative counts | -| 2 | Replace two slow-path options with one: `RecordValueWithCount(fromHistogram.ValueFromIndex(i), -count)` | -| 3 | Drop `if (count != 0)` guard from slow-path to match `Add` | -| 4 | Specify Test 1 setup and assertions with exact values and `Assert.Equal` calls | -| 5 | Decide whether to copy the existing `Add` exception message or fix it in both methods; document the choice | - -None of these change the scope or architecture. -After addressing them the brief is ready to move to `plan/ready/`. diff --git a/plan/planning/brief.md b/plan/planning/brief.md index 1d415a1..cf81dbd 100644 --- a/plan/planning/brief.md +++ b/plan/planning/brief.md @@ -12,7 +12,7 @@ This is a natural complement to `Add` and follows the same structural pattern (f | File | Change | |---|---| -| `HdrHistogram/HistogramBase.cs` | Add `Subtract(HistogramBase)` virtual method (primary 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` | @@ -34,14 +34,14 @@ Concrete implementations (`LongHistogram`, `IntHistogram`, `ShortHistogram`, `Lo 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 values into `histogram` and `other` (same config). - - Record one extra value into `histogram` at the same slots. - - Subtract `other` from `histogram`. - - Assert that counts equal the surplus, and `TotalCount` is correct. + - 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 smaller histogram from `biggerOther`. + - Subtract the smaller histogram from `biggerOther`. - Assert counts and `TotalCount` decrease correctly. 3. **`Subtract_throws_if_other_has_a_larger_range`** @@ -52,60 +52,55 @@ These three tests run against all concrete histogram types via the existing abst ## Implementation Notes -### Fast path +### 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: ```csharp -for (var i = 0; i < fromHistogram.CountsArrayLength; i++) +if (HighestTrackableValue < fromHistogram.HighestTrackableValue) { - AddToCountAtIndex(i, -fromHistogram.GetCountAtIndex(i)); + throw new ArgumentOutOfRangeException( + nameof(fromHistogram), + $"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue}) than this one ({HighestTrackableValue})."); } ``` -`AddToCountAtIndex` already accepts negative `addend` values; passing the negated count is sufficient. +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`. -### Slow path +### Fast path ```csharp for (var i = 0; i < fromHistogram.CountsArrayLength; i++) { - var count = fromHistogram.GetCountAtIndex(i); - if (count != 0) - { - SubtractFromCountAtValue(fromHistogram.ValueFromIndex(i), count); - } + AddToCountAtIndex(i, -fromHistogram.GetCountAtIndex(i)); } ``` -The slow path requires a helper `SubtractFromCountAtValue` (analogous to `RecordValueWithCount` but decrements). -Alternatively — and more simply — reuse `RecordValueWithCount` with a **negated** count if the internal implementation tolerates it (it calls `AddToCountAtIndex` which does). -Confirm this before coding; if `RecordValueWithCount` has a guard rejecting negative counts, a dedicated helper is needed. - -### Validation +`AddToCountAtIndex` already accepts negative `addend` values; passing the negated count is sufficient. -Identical guard to `Add`: +### Slow path ```csharp -if (HighestTrackableValue < fromHistogram.HighestTrackableValue) +for (var i = 0; i < fromHistogram.CountsArrayLength; i++) { - throw new ArgumentOutOfRangeException( - nameof(fromHistogram), - $"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue}) " + - $"than this one ({HighestTrackableValue})."); + 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. **`RecordValueWithCount` with negative count**: Check whether `RecordValueWithCount` internally rejects negative values. - If it does, the slow path must call `AddToCountAtIndex` directly (after computing the index) rather than going via `RecordValueWithCount`. - -3. **Concurrent implementations**: `LongConcurrentHistogram` and `IntConcurrentHistogram` override `AddToCountAtIndex` with atomic operations. +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. -4. **`TotalCount` consistency**: `AddToCountAtIndex` increments `TotalCount` by the addend. +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. From 007c10a389ebf9053923245d93b9209b3fa32d7d Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 2 Mar 2026 00:43:42 +0000 Subject: [PATCH 4/7] plan(#82): review brief --- plan/{planning => ready}/brief.md | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename plan/{planning => ready}/brief.md (100%) diff --git a/plan/planning/brief.md b/plan/ready/brief.md similarity index 100% rename from plan/planning/brief.md rename to plan/ready/brief.md From b99ae865b6f53b36addf215fb3ffd91d418c943e Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 2 Mar 2026 00:45:42 +0000 Subject: [PATCH 5/7] plan(#82): create task breakdown --- plan/ready/task.md | 102 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) create mode 100644 plan/ready/task.md diff --git a/plan/ready/task.md b/plan/ready/task.md new file mode 100644 index 0000000..5512640 --- /dev/null +++ b/plan/ready/task.md @@ -0,0 +1,102 @@ +# 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: ``, ``, `` + - 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` | From a8a42a183640b8e8684b4130c192e1ab98abe3b2 Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 2 Mar 2026 00:52:42 +0000 Subject: [PATCH 6/7] feat(#82): implement tasks --- HdrHistogram.UnitTests/HistogramTestBase.cs | 45 +++++++++++++++++++++ HdrHistogram/HistogramBase.cs | 39 +++++++++++++++++- plan/{ready => done}/brief.md | 0 plan/{ready => done}/task.md | 14 +++---- spec/tech-standards/api-reference.md | 1 + 5 files changed, 90 insertions(+), 9 deletions(-) rename plan/{ready => done}/brief.md (100%) rename plan/{ready => done}/task.md (93%) diff --git a/HdrHistogram.UnitTests/HistogramTestBase.cs b/HdrHistogram.UnitTests/HistogramTestBase.cs index f1d15fd..f07b7d1 100644 --- a/HdrHistogram.UnitTests/HistogramTestBase.cs +++ b/HdrHistogram.UnitTests/HistogramTestBase.cs @@ -259,6 +259,51 @@ public void Add_throws_if_other_has_a_larger_range() Assert.Throws(() => { histogram.Add(biggerOther); }); } + [Fact] + public void Subtract_should_reduce_the_counts_from_two_histograms() + { + var histogram = Create(DefaultHighestTrackableValue, DefaultSignificantFigures); + var other = Create(DefaultHighestTrackableValue, DefaultSignificantFigures); + histogram.RecordValue(TestValueLevel); + histogram.RecordValue(TestValueLevel); + histogram.RecordValue(TestValueLevel * 1000); + histogram.RecordValue(TestValueLevel * 1000); + other.RecordValue(TestValueLevel); + other.RecordValue(TestValueLevel * 1000); + + histogram.Subtract(other); + + Assert.Equal(1L, histogram.GetCountAtValue(TestValueLevel)); + Assert.Equal(1L, histogram.GetCountAtValue(TestValueLevel * 1000)); + Assert.Equal(2L, histogram.TotalCount); + } + + [Fact] + public void Subtract_should_allow_small_range_histograms_to_be_subtracted() + { + var biggerOther = Create(DefaultHighestTrackableValue * 2, DefaultSignificantFigures); + var histogram = Create(DefaultHighestTrackableValue, DefaultSignificantFigures); + biggerOther.RecordValue(TestValueLevel); + biggerOther.RecordValue(TestValueLevel * 1000); + histogram.RecordValue(TestValueLevel); + histogram.RecordValue(TestValueLevel * 1000); + + biggerOther.Subtract(histogram); + + Assert.Equal(0L, biggerOther.GetCountAtValue(TestValueLevel)); + Assert.Equal(0L, biggerOther.GetCountAtValue(TestValueLevel * 1000)); + Assert.Equal(0L, biggerOther.TotalCount); + } + + [Fact] + public void Subtract_throws_if_other_has_a_larger_range() + { + var histogram = Create(DefaultHighestTrackableValue, DefaultSignificantFigures); + var biggerOther = Create(DefaultHighestTrackableValue * 2, DefaultSignificantFigures); + + Assert.Throws(() => { histogram.Subtract(biggerOther); }); + } + [Theory] [InlineData(1, 1)] [InlineData(2, 2500)] diff --git a/HdrHistogram/HistogramBase.cs b/HdrHistogram/HistogramBase.cs index 194fb48..0e90363 100644 --- a/HdrHistogram/HistogramBase.cs +++ b/HdrHistogram/HistogramBase.cs @@ -280,7 +280,7 @@ public virtual void Add(HistogramBase fromHistogram) { if (HighestTrackableValue < fromHistogram.HighestTrackableValue) { - throw new ArgumentOutOfRangeException(nameof(fromHistogram), $"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue} than this one ({HighestTrackableValue})."); + throw new ArgumentOutOfRangeException(nameof(fromHistogram), $"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue}) than this one ({HighestTrackableValue})."); } if ((BucketCount == fromHistogram.BucketCount) && (SubBucketCount == fromHistogram.SubBucketCount) && @@ -305,7 +305,42 @@ public virtual void Add(HistogramBase fromHistogram) } /// - /// Get the size (in value units) of the range of values that are equivalent to the given value within the histogram's resolution. + /// Subtract the contents of another histogram from this one. + /// + /// The other histogram. + /// if fromHistogram covers a wider range than this histogram. + public virtual void Subtract(HistogramBase fromHistogram) + { + if (HighestTrackableValue < fromHistogram.HighestTrackableValue) + { + throw new ArgumentOutOfRangeException( + nameof(fromHistogram), + $"The other histogram covers a wider range ({fromHistogram.HighestTrackableValue}) than this one ({HighestTrackableValue})."); + } + if ((BucketCount == fromHistogram.BucketCount) && + (SubBucketCount == fromHistogram.SubBucketCount) && + (_unitMagnitude == fromHistogram._unitMagnitude)) + { + // Counts arrays are of the same length and meaning, so we can just iterate and subtract directly: + for (var i = 0; i < fromHistogram.CountsArrayLength; i++) + { + AddToCountAtIndex(i, -fromHistogram.GetCountAtIndex(i)); + } + } + else + { + // Arrays are not a direct match, so we can't just stream through and subtract them. + // Instead, go through the array and subtract each non-zero value found at its proper value: + for (var i = 0; i < fromHistogram.CountsArrayLength; i++) + { + var count = fromHistogram.GetCountAtIndex(i); + RecordValueWithCount(fromHistogram.ValueFromIndex(i), -count); + } + } + } + + /// + /// Get the size (in value units) of the range of values that are equivalent to the given value within the histogram's resolution. /// Where "equivalent" means that value samples recorded for any two equivalent values are counted in a common total count. /// /// The given value diff --git a/plan/ready/brief.md b/plan/done/brief.md similarity index 100% rename from plan/ready/brief.md rename to plan/done/brief.md diff --git a/plan/ready/task.md b/plan/done/task.md similarity index 93% rename from plan/ready/task.md rename to plan/done/task.md index 5512640..9d39268 100644 --- a/plan/ready/task.md +++ b/plan/done/task.md @@ -2,7 +2,7 @@ ## Implementation -- [ ] **Fix malformed exception message in `Add` in `HistogramBase.cs`** +- [x] **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})."` @@ -10,7 +10,7 @@ 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`** +- [x] **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)` @@ -27,7 +27,7 @@ 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`** +- [x] **Add XML doc comment to `Subtract`** - File: `HdrHistogram/HistogramBase.cs`, directly above the new `Subtract` method - Required elements: ``, ``, `` - Why: The brief's acceptance criterion requires documentation matching the quality of the @@ -36,7 +36,7 @@ ## Tests -- [ ] **Add `Subtract_should_reduce_the_counts_from_two_histograms` to `HistogramTestBase.cs`** +- [x] **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: @@ -51,7 +51,7 @@ 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`** +- [x] **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`. @@ -65,7 +65,7 @@ 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`** +- [x] **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`. @@ -77,7 +77,7 @@ ## Documentation -- [ ] **Document `Subtract` in `spec/tech-standards/api-reference.md`** +- [x] **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 diff --git a/spec/tech-standards/api-reference.md b/spec/tech-standards/api-reference.md index 1e8ee5e..dd57810 100644 --- a/spec/tech-standards/api-reference.md +++ b/spec/tech-standards/api-reference.md @@ -138,6 +138,7 @@ long GetCountBetweenValues(long lowValue, long highValue) ```csharp void Add(HistogramBase other) // Merge histograms +void Subtract(HistogramBase other) // Remove histogram values HistogramBase Copy() // Create a copy void Reset() // Clear all counts bool HasOverflowed() // Check for overflow From dc2dea2d4b06022df86d59d231545f3bef1e30fc Mon Sep 17 00:00:00 2001 From: leecampbell-codeagent Date: Mon, 2 Mar 2026 00:53:14 +0000 Subject: [PATCH 7/7] feat(#82): complete implementation --- plan/done/brief.md | 106 --------------------------------------------- plan/done/task.md | 102 ------------------------------------------- 2 files changed, 208 deletions(-) delete mode 100644 plan/done/brief.md delete mode 100644 plan/done/task.md diff --git a/plan/done/brief.md b/plan/done/brief.md deleted file mode 100644 index cf81dbd..0000000 --- a/plan/done/brief.md +++ /dev/null @@ -1,106 +0,0 @@ -# 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: - -```csharp -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 - -```csharp -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 - -```csharp -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. diff --git a/plan/done/task.md b/plan/done/task.md deleted file mode 100644 index 9d39268..0000000 --- a/plan/done/task.md +++ /dev/null @@ -1,102 +0,0 @@ -# Task List: Issue #82 — Add `Subtract` Method to `HistogramBase` - -## Implementation - -- [x] **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. - -- [x] **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. - -- [x] **Add XML doc comment to `Subtract`** - - File: `HdrHistogram/HistogramBase.cs`, directly above the new `Subtract` method - - Required elements: ``, ``, `` - - 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 - -- [x] **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. - -- [x] **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. - -- [x] **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 - -- [x] **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` |