refactor: resolve high coupling, God Class, and high cognitive complexity in core library classes - #231
Open
sajibsarkar92 wants to merge 7 commits into
Open
Conversation
…eDiffAnnotator, InlineDiffAnnonatorConfig, InlineTagRender.java
…5 via phase-driven helpers
There was a problem hiding this comment.
Pull request overview
This PR refactors core java-diff-utils classes to reduce coupling and cognitive complexity by extracting patching, inline diff annotation, delta normalization, and unified-diff parsing phases into focused helpers while keeping existing behavior intact.
Changes:
- Split patching and inline-diff helpers into
PatchUtilsandInlineDiffUtils, and introducedDiffAlgorithmDefaultsfor default algorithm selection. - Decomposed
DiffRowGeneratorresponsibilities into dedicated helpers (InlineDiffAnnotator,InlineTagRenderer,DeltaDecompressor, and config object). - Refactored
UnifiedDiffReader.parse()into phase methods to reduce nesting and repeated rule lists.
Reviewed changes
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Formatting/lifecycle mapping adjustments (Spotless/m2e related) |
| java-diff-utils/src/main/java/com/github/difflib/DiffUtils.java | Slimmed diff façade; default algorithm now via DiffAlgorithmDefaults |
| java-diff-utils/src/main/java/com/github/difflib/PatchUtils.java | New patch/unpatch utility wrapper around Patch |
| java-diff-utils/src/main/java/com/github/difflib/InlineDiffUtils.java | New inline character-level diff utility |
| java-diff-utils/src/main/java/com/github/difflib/DiffAlgorithmDefaults.java | New default diff algorithm factory provider |
| java-diff-utils/src/main/java/com/github/difflib/unifieddiff/UnifiedDiffReader.java | Refactored parsing into header/file/chunk/tail phases |
| java-diff-utils/src/main/java/com/github/difflib/text/DiffRowGenerator.java | Reduced to orchestrator; delegates inline annotation and delta decompression |
| java-diff-utils/src/main/java/com/github/difflib/text/InlineTagRenderer.java | Extracted tag injection logic used by inline annotation |
| java-diff-utils/src/main/java/com/github/difflib/text/DeltaDecompressor.java | Extracted asymmetric ChangeDelta normalization logic |
| java-diff-utils/src/main/java/com/github/difflib/text/InlineDiffAnnotator.java | New dedicated inline token diff + tag application engine |
| java-diff-utils/src/main/java/com/github/difflib/text/InlineDiffAnnotatorConfig.java | New config carrier for inline annotator execution |
| java-diff-utils/src/test/java/com/github/difflib/unifieddiff/UnifiedDiffRoundTripTest.java | Updated patch application calls to PatchUtils |
| java-diff-utils/src/test/java/com/github/difflib/patch/PatchWithMyerDiffWithLinearSpaceTest.java | Updated patch application calls to PatchUtils |
| java-diff-utils/src/test/java/com/github/difflib/patch/PatchWithMyerDiffTest.java | Updated patch application calls to PatchUtils |
| java-diff-utils/src/test/java/com/github/difflib/patch/PatchWithAllDiffAlgorithmsTest.java | Updated patch application calls to PatchUtils |
| java-diff-utils/src/test/java/com/github/difflib/GenerateUnifiedDiffTest.java | Updated patch application calls to PatchUtils |
| java-diff-utils/src/test/java/com/github/difflib/examples/ApplyPatch.java | Updated example to use PatchUtils |
| java-diff-utils/src/test/java/com/github/difflib/DiffUtilsTest.java | Updated inline diff tests to use InlineDiffUtils |
| java-diff-utils/src/test/java/com/github/difflib/algorithm/myers/WithMyersDiffWithLinearSpacePatchTest.java | Updated patch application calls to PatchUtils |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
sajibsarkar92
marked this pull request as draft
July 4, 2026 08:48
sajibsarkar92
marked this pull request as ready for review
July 4, 2026 08:53
…tibility and fix PMD violations
sajibsarkar92
force-pushed
the
refactor/code-smell-fixes
branch
from
July 4, 2026 09:17
749d76e to
bbf6ec6
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor: Resolve High Coupling, God Class, and High Cognitive Complexity
This PR addresses three critical architectural code smells identified via PMD static analysis.
All changes are purely structural refactorings — zero changes to public APIs, method
signatures, or observable behavior.
Issues Addressed
1. High Object Coupling in
DiffUtils.java(CBO Score: 29)Problem:
DiffUtilswas directly coupled to 29 internal classes simultaneously, actingas a monolithic control panel for algorithm engines, patch containers, and formatting
utilities. This made future algorithm changes risky and violated separation of concerns.
Fix — Delegation Pattern:
PatchUtils(new) — owns all patch apply / unpatch / merge logicInlineDiffUtils(new) — owns character and word-level inline annotationDiffAlgorithmDefaults(new) — factory for default algorithm engine instancesDiffUtilsis now a slim façade that delegates to these specialists2. God Class in
DiffRowGenerator.java(~780 lines, SRP Violation)Problem: A single 780-line class simultaneously managed diff orchestration, HTML tag
rendering, and asymmetric delta normalization — three unrelated concerns that violated the
Single Responsibility Principle.
Fix — Single-Responsibility Decomposition:
InlineTagRenderer(new) — isolates HTML span tag injection and newline boundary handlingDeltaDecompressor(new) — normalizes asymmetric ChangeDelta objects (e.g. 3 deletedlines vs 1 inserted) into uniform 1-to-1 row pairs for table rendering
InlineDiffAnnotatorConfig(new) — immutable value object bundling all annotationconfiguration flags, eliminating parameter bloat
InlineDiffAnnotator(new) — centralized token-level word/character highlighting engineDiffRowGeneratorreduced from ~780 lines to ~240 lines; now a pure orchestrator3. High Cognitive Complexity in
UnifiedDiffReader.java(Score: 45 reduced to ~5)Problem: The
parse()method had a Cognitive Complexity score of 45 — three times theallowed limit of 15 — caused by 4 levels of deeply nested while/if blocks and 16-parameter
vararg rule lists repeated across methods.
Fix — Phase-Driven Assembly Line Pattern:
parseHeaderSection()(new) — reads global preamble before any file diffparseFileHeader()(new) — consumes file metadata (---, +++, git index lines)parseChunkSection()andisChunkFinished()(new) — processes @@ chunk headersand line-by-line additions/deletions
parseTailSection()(new) — handles trailing footer content after all file diffsparse()is now a flat, readable ~15-line sequential orchestratorFiles Changed
DiffUtils.javaPatchUtils.javaInlineDiffUtils.javaDiffAlgorithmDefaults.javaDiffRowGenerator.javaInlineTagRenderer.javaDeltaDecompressor.javaInlineDiffAnnotator.javaInlineDiffAnnotatorConfig.javaUnifiedDiffReader.javapom.xml(parent)Design Principles Applied
Functional Validation
All existing tests pass with zero regressions. The 3 skipped tests are pre-existing
upstream skips entirely unrelated to this refactoring.