uPortal-Project/uPortal

Refactor Potential Implementation and Design Smells

Opened this issue · 2 comments

Problem

This pull request will use strategic refactoring techniques in the uPortal open-source project to enhance code readability, maintainability, and extensibility.

Solution

These refactorings aim to elevate the quality of the uPortal codebase, making it more maintainable, understandable, and extensible for both current and future contributors.

I have tried my best to apply the below refactoring techniques: (each commit will depict that technique in my PR)

To cater Implementations smells

  1. Extract Method Applied
  2. Rename Method applied for better clarity
  3. Decomposed Conditional to reduce the cyclomatic complexity.

To cater Design Smells:

  1. Move Method Refactoring Applied to cater feature envy
  2. Pull Method / Variable Method applied to remove code duplicate (where encountered)
  3. Replaced Conditional with Polymorphism.

Alternatives

There can be multiple design choices made to make the code more maintainable. Due to trade-offs and complexity, I have not YET used the extract class refactoring yet. There are some monster classes which demonstrate insufficient modularization. Currently, I have used a few of the refactoring as answered above.

Hey @snehit221! 👋

Improvements to code quality and reducing code smells is welcome!
There are a number of tools in place which help with this spotbugs, Error Prone, CodeNarc, ESLint, and stylelint to name a few.
Which could point to areas to focus on.

With any refactor, consider focusing any given PR on a single type of refactor/smell at a time, and applying it module by module if possible.
This both makes changes easier to review, and reduces risk of work being lost/unused if it doesn't fit with the project.


Extract Method Applied

Is there a specific section of code you believe needs to be extracted?
I'm certainly open to it, but don't necessarily want to apply DRY for the sake of being DRY, instead looking for deeper organizational improvements https://medium.com/@kiwiandroiddev/beyond-dry-why-redundancy-makes-your-code-more-robust-and-less-fragile-6a64d0172a57

Rename Method applied for better clarity

Again are there particular sections where you've found names which are confusing or incorrect?
With internals there's plenty of flexibility with naming, for public APIs, we'd want to approach that with more caution to avoid breaking adopters.

Decomposed Conditional to reduce the cyclomatic complexity

If there are good abstractions they can certainly be applied.
There are also parts which have irreducible complexity, breaking those apart for the sake of hitting a cyclomatic complexity target will reduce readability.

Move Method Refactoring Applied to cater feature envy
Pull Method / Variable Method applied to remove code duplicate (where encountered)
Replaced Conditional with Polymorphism.

This is a general list of code smells.
It's a bit vague what specifically you are proposing here.

Hi @ChristianMurphy,

Thanks for your valuable inputs and guidance. I went ahead and used tools such as Designite and SonarQube, which gave me some clues about possible refactoring. I have created two pull requests, where each commit shows the type of refactoring I applied to improve code maintainability.

I have tried to keep each commit relatively simple from my end.
As for the move method, pull method and replaced conditional
I have added the commit message where ever I applied that. I think it would be easier to comprehend once the PR is looked at.

My primary goal was to apply these implementation and design level refactoring as a starter, so the the code base can be more flexible, maintainable and closely follows SOLID design.