Skip to content

Refactor/smell free solid clean code#7514

Open
nabila1644-iit wants to merge 12 commits into
TheAlgorithms:masterfrom
nabila1644-iit:refactor/smell-free-solid-clean-code
Open

Refactor/smell free solid clean code#7514
nabila1644-iit wants to merge 12 commits into
TheAlgorithms:masterfrom
nabila1644-iit:refactor/smell-free-solid-clean-code

Conversation

@nabila1644-iit

Copy link
Copy Markdown

Title

Refactor BronKerbosch.java to resolve SRP/OCP/DIP violations and Bloater/Coupler smells

What issues were addressed

  • Long Method: findMaximalCliques mixed validation, graph copying, and algorithm init in one method
  • Feature Envy: bronKerboschPivot and helpers were static utilities not tied to any object
  • Primitive Obsession: raw List<Set<Integer>> used instead of a typed graph abstraction
  • Data Clumps: r, p, x always traveled together without being packaged as a unit
  • SRP/OCP/DIP violations: one class handled validation, construction, pivot logic, and enumeration; pivot strategy was hardcoded; logic depended on concrete HashSet/ArrayList types

Why these changes improve the system

  • Splitting responsibilities (Graph, GraphValidator, PivotStrategy, SearchState, Solver, façade) means each class now has exactly one reason to change
  • Pivot selection is now swappable via PivotStrategy without touching the algorithm (OCP)
  • The solver depends on the Graph interface, not concrete collection types (DIP)
  • SearchState replaces the r/p/x data clump with an immutable, self-validating value object

Summary of refactoring approach

  • Extracted a Graph interface + AdjacencyListGraph implementation to decouple the algorithm from raw collections
  • Extracted GraphValidator to isolate input-validation logic
  • Introduced PivotStrategy interface with MaxDegreePivotStrategy as the default implementation
  • Replaced mutable shared state with an immutable SearchState value object, deriving new states via expandWith/processVertex instead of in-place mutation
  • Kept the original public API (BronKerbosch.findMaximalCliques(List<Set<Integer>>)) unchanged as a façade over the new internal design

Functionality assurance

  • Public method signature, return type, and algorithm logic (Bron–Kerbosch with pivoting) are unchanged
  • Recursive mutation sequence of the original r/p/x sets was manually traced against the new immutable state transitions to confirm identical clique enumeration
  • No test suite exists for this class in the repository (educational reference project), so verification was done via manual trace rather than automated tests — noted transparently for reviewers

Nabila and others added 11 commits June 25, 2026 20:29
… I/O (SRP), remove unused processes array, add input validation and try-with-resources
… and rendering to resolve SRP/OCP/DIP violations, remove duplicate code and hardcoded constants
…te, and Solver abstractions to resolve SRP/OCP/DIP violations and Long Method/Feature Envy/Data Clumps smells
…e code, depend on List interface internally while preserving ArrayList return type for backward compatibility
…, and Trie access to resolve SRP violation and Inappropriate Intimacy smell
Copilot AI review requested due to automatic review settings July 5, 2026 14:13

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.

Copilot wasn't able to review this pull request because it exceeds the maximum number of files (300). Try reducing the number of changed files and requesting a review from Copilot again.

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