← 返回首页
refactor: resolve high coupling, God Class, and high cognitive complexity in core library classes by sajibsarkar92 · Pull Request #231 · java-diff-utils/java-diff-utils · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
Resetting focus

refactor: resolve high coupling, God Class, and high cognitive complexity in core library classes#231

Open
sajibsarkar92 wants to merge 7 commits into
java-diff-utils:masterfrom
sajibsarkar92:refactor/code-smell-fixes
Open

refactor: resolve high coupling, God Class, and high cognitive complexity in core library classes#231
sajibsarkar92 wants to merge 7 commits into
java-diff-utils:masterfrom
sajibsarkar92:refactor/code-smell-fixes

Conversation

Copy link
Copy Markdown

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: DiffUtils was directly coupled to 29 internal classes simultaneously, acting
as 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 logic
  • InlineDiffUtils (new) — owns character and word-level inline annotation
  • DiffAlgorithmDefaults (new) — factory for default algorithm engine instances
  • DiffUtils is now a slim façade that delegates to these specialists

2. 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 handling
  • DeltaDecompressor (new) — normalizes asymmetric ChangeDelta objects (e.g. 3 deleted
    lines vs 1 inserted) into uniform 1-to-1 row pairs for table rendering
  • InlineDiffAnnotatorConfig (new) — immutable value object bundling all annotation
    configuration flags, eliminating parameter bloat
  • InlineDiffAnnotator (new) — centralized token-level word/character highlighting engine
  • DiffRowGenerator reduced from ~780 lines to ~240 lines; now a pure orchestrator

3. High Cognitive Complexity in UnifiedDiffReader.java (Score: 45 reduced to ~5)

Problem: The parse() method had a Cognitive Complexity score of 45 — three times the
allowed 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:

  • Consolidated 16 regex rule varargs into pre-allocated static arrays
  • parseHeaderSection() (new) — reads global preamble before any file diff
  • parseFileHeader() (new) — consumes file metadata (---, +++, git index lines)
  • parseChunkSection() and isChunkFinished() (new) — processes @@ chunk headers
    and line-by-line additions/deletions
  • parseTailSection() (new) — handles trailing footer content after all file diffs
  • parse() is now a flat, readable ~15-line sequential orchestrator

Files Changed

File Change Description
DiffUtils.java Modified Converted to slim delegating façade
PatchUtils.java New Owns all patch manipulation logic
InlineDiffUtils.java New Owns inline diff annotation logic
DiffAlgorithmDefaults.java New Algorithm engine factory
DiffRowGenerator.java Modified Reduced to ~240-line orchestrator
InlineTagRenderer.java New HTML tag injection specialist
DeltaDecompressor.java New Asymmetric delta normalization
InlineDiffAnnotator.java New Token-level highlighting engine
InlineDiffAnnotatorConfig.java New Immutable annotation config object
UnifiedDiffReader.java Modified Phase-driven complexity reduction
pom.xml (parent) Modified Eclipse m2e lifecycle mapping for Spotless

Design Principles Applied

  • Single Responsibility Principle — each new class has exactly one job
  • Delegation Pattern — façades forward to specialists instead of doing everything
  • Open/Closed Principle — new algorithms or renderers can be added without touching core façades
  • Backward Compatibility — all public method signatures fully preserved

Functional Validation

All existing tests pass with zero regressions. The 3 skipped tests are pre-existing
upstream skips entirely unrelated to this refactoring.

Tests run: 11, Failures: 0, Errors: 0 — GenerateUnifiedDiffTest Tests run: 1, Failures: 0, Errors: 0 — ChunkTest Tests run: 8, Failures: 0, Errors: 0 — PatchWithAllDiffAlgorithmsTest Tests run: 2, Failures: 0, Errors: 0 — PatchWithMyerDiffTest Tests run: 1, Failures: 0, Errors: 0 — PatchWithMyerDiffWithLinearSpaceTest Tests run: 4, Failures: 0, Errors: 0 — DiffRowGeneratorEqualitiesTest Tests run: 44, Failures: 0, Errors: 0 — DiffRowGeneratorTest Tests run: 4, Failures: 0, Errors: 0 — StringUtilsTest Tests run: 32, Failures: 0, Errors: 0 — UnifiedDiffReaderTest Tests run: 7, Failures: 0, Errors: 0 — UnifiedDiffRoundTripTest (1 pre-existing skip) Tests run: 1, Failures: 0, Errors: 0 — UnifiedDiffRoundTripNewLineTest (1 pre-existing skip) Tests run: 2, Failures: 0, Errors: 0 — UnifiedDiffWriterTest Tests run: 2, Failures: 0, Errors: 0 — HistogramDiffTest (jgit module) Total — Tests run: 147, Failures: 0, Errors: 0, Skipped: 3 BUILD SUCCESS

Copilot AI review requested due to automatic review settings July 4, 2026 08:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide 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 PatchUtils and InlineDiffUtils, and introduced DiffAlgorithmDefaults for default algorithm selection.
  • Decomposed DiffRowGenerator responsibilities 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.

Comment thread java-diff-utils/src/main/java/com/github/difflib/DiffUtils.java Show resolved Hide resolved
Comment thread java-diff-utils/src/main/java/com/github/difflib/PatchUtils.java Show resolved Hide resolved
Comment thread java-diff-utils/src/main/java/com/github/difflib/PatchUtils.java Show resolved Hide resolved
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
sajibsarkar92 force-pushed the refactor/code-smell-fixes branch from 749d76e to bbf6ec6 Compare July 4, 2026 09:17
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Footer

© 2026 GitHub, Inc.