Skip to content

Commit 66d01c4

Browse files
Rylan-cgiclaude
andcommitted
refactor(schedule10): split document assembly out of Schedule10Service
Clears the last two SonarCloud findings on PR #305. USE containsEntry (Minor). One assertion was left on the old shape by the earlier mechanical pass, because it spanned lines and carried an .as(...) description. Converted, and folded together with the assertion below it so the pair cannot return as a "join assertions" flag. MONSTER CLASS (21 dependencies against a limit of 20). Sonar's suggested fix was to split the class, and that is what this does rather than trimming one incidental dependency to get under the threshold. Schedule10Service had grown to 1137 lines doing two genuinely separate jobs, and the seam between them was clean: nothing in the assembly touches a request DTO, and nothing in the write path touches a response DTO beyond returning one. Schedule10Service 1137 -> 786 lines the seven writes, validation, Check Status entry point Schedule10DocumentAssembler 378 lines (new) assembly and read-side derivation Schedule10CostItems 52 lines (new) the 12 cost-item ordinals and 3 column scales BOTH halves need Service imports fall from 38 to 23 -- fifteen became unused the moment the assembly left, which is precisely the coupling Sonar was measuring. Schedule10CostItems exists because the ordinals and scales are genuinely shared; the alternatives were declaring the same twelve magic numbers twice, or having the assembler reach back into the service. This also closes a separate code-review finding at no extra cost. The write paths previously read the track status TWICE -- once for requireDraft's gate, then again inside assembly to compute editable. The service now reads it once and passes both the status and the derived editability to the assembler, so there is one read per request. Safe to change: the service test uses lenient mocks with no call-count verification. The assembler is constructed by the service rather than injected as a bean. It is an implementation detail with no independent lifecycle, and constructing it keeps the existing single-argument constructor every test already uses, so no test wiring changed. Verification: 1151 unit + 881 integration tests, 0 failures; 0 checkstyle violations in schedule10. Schedule10DocumentIT -- which asserts the assembled document byte-for-byte against the pinned contract, and covers the code that moved -- passes unchanged, which is the evidence that the extraction preserved behaviour. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent dbd0a19 commit 66d01c4

4 files changed

Lines changed: 515 additions & 400 deletions

File tree

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
package ca.bc.gov.nrs.ilcr.schedule10;
2+
3+
import java.util.Set;
4+
5+
/**
6+
* The legacy cost-item ordinals Schedule 10 routes, and the column scales its dimensions are stored
7+
* at.
8+
*
9+
* <p>Held here because BOTH halves of the schedule need them: the read path routes stored cost rows
10+
* into the document's substructures by ordinal, and the write path writes the same twelve rows
11+
* back. They lived on {@code Schedule10Service} until the document assembly was extracted (code
12+
* review follow-up 2026-08-19) — at which point keeping them there would have meant either a
13+
* back-reference from the assembler to the service, or the same twelve magic numbers declared
14+
* twice.
15+
*/
16+
final class Schedule10CostItems {
17+
18+
// Legacy cost-item ordinals (Constant.REPORT_COST_ITEMS :371-376), all verified against the
19+
// delivery ILCR_REPORT_COST_ITEM rows. The six "Less" lines span THREE subcategories, so routing
20+
// must be by item id — scanning a single subcategory would silently under-count the deductions.
21+
static final int SUB_GRADE_TRANSFER = 3; // cat 10 / sub 1
22+
static final int LESS_OTHER_ENGINEERING = 4; // cat 10 / sub 3
23+
static final int OTHER_TT_TRANSFER = 5; // cat 10 / sub 3
24+
static final int LESS_CULVERT = 6; // cat 10 / sub 1
25+
static final int LESS_BRIDGE = 7; // cat 10 / sub 1
26+
static final int LESS_LANDING = 8; // cat 10 / sub 1
27+
static final int STABILIZING_OTHER_TRANSFER = 9; // cat 10 / sub 4
28+
static final int STABILIZING_TRANSFER = 10; // cat 10 / sub 2
29+
static final int LESS_OVERLAND = 11; // cat 10 / sub 1
30+
static final int SUB_GRADE_ACTUAL = 20; // cat 10 / sub 1
31+
static final int LESS_END_HAUL = 21; // cat 10 / sub 1
32+
static final int STABILIZING_ACTUAL = 22; // cat 10 / sub 2
33+
34+
/**
35+
* Every cost-item ordinal Schedule 10 routes. A cost row outside this set contributes to no
36+
* substructure and would silently vanish from the totals, so it is logged instead.
37+
*/
38+
static final Set<Integer> ROUTED = Set.of(
39+
SUB_GRADE_TRANSFER, LESS_OTHER_ENGINEERING, OTHER_TT_TRANSFER, LESS_CULVERT, LESS_BRIDGE,
40+
LESS_LANDING, STABILIZING_OTHER_TRANSFER, STABILIZING_TRANSFER, LESS_OVERLAND,
41+
SUB_GRADE_ACTUAL, LESS_END_HAUL, STABILIZING_ACTUAL);
42+
43+
// G8 — Oracle does not preserve trailing zeros, so a NUMBER(6,3) holding 3.000 comes back as 3
44+
// and serialises as the integer 3 while its 12.500 neighbour serialises as 12.5. Stored
45+
// dimensions are normalised to their column's declared scale so the served document matches the
46+
// pinned contract regardless of the value (code review 2026-08-17 — caught by a new assertion).
47+
static final int LENGTH_SCALE = 3; // SUB_GRADE_LENGTH / STABILIZING_LENGTH NUMBER(6,3)
48+
static final int MEASURE_SCALE = 1; // widths, depth, distances NUMBER(x,1)
49+
static final int VOLUME_SCALE = 0; // END_HAUL_VOLUME / OVERLAND_VOLUME NUMBER(7,0)
50+
51+
private Schedule10CostItems() {
52+
}
53+
}

0 commit comments

Comments
 (0)