Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove code smells and improve test coverage #33

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

momo1606
Copy link

Refactored code to remove various Implementation and Design smells from various files and improved test coverage.
Added 6 new unit tests -
JSONSignalStoreTest (4 tests)
SegmentInputStreamTest (2 tests)

Refactoring Techniques used to remove smells and improve readability-

  1. Class name - NoiseInputStream
    Method - readDecryptedSegment
    Used explaining variable to remove Magic Number

  2. Package - nl.ben221199.wapi
    Class - JSONSignalStore
    Method - getJSON
    Cyclomatic complexity of the method is 10
    Refactored using Extract Method to remove Complex Method smell

  3. Refactoring - Rename method/variable
    Package - com.southernstorm.noise.crypto
    Class- NewHope
    Method name - f() and g()
    From line- 539
    Smells removed - Magic Number

  4. Refactoring- Decompose Conditional
    Package - com.southernstorm.noise.protocol
    Class- HandshakeState
    Method- fallback
    Method Line- 632
    The conditional expression (action != FAILED && action != READ_MESSAGE) || !localEphemeral.hasPublicKey() is complex.
    And the conditional expression (action != FAILED && action != WRITE_MESSAGE) || !remoteEphemeral.hasPublicKey() is
    complex.

  5. Refactoring - Pull-up method/variable
    Package- nl.ben221199.wapi.fun.token
    Class 1- ShortList
    Class 1 method line - 9
    Class 2- LongList
    Class 2 method line - 9
    Both the classes extend the class AbstractList, and no other class extends AbstractList. There is a variable ‘length’ in
    both the subclasses ShortList and LongList which can be taken common into the AbstractList class.

  6. Refactoring- Replace conditional with polymorphism
    Package - nl.ben221199.wapi
    Class- Verification
    Method- calculateToken
    Method Line- 115
    The method has a switch statement that calculates a token based on the specified platform in the Verification class. This
    code violates the open/closed principle, as adding support for a new platform requires modifying the existing code.

  7. Refactoring - Extract Class
    Package - nl.ben221199.wapi
    Class- Connection
    The Connection class exhibits the multifaceted abstraction smell. It combines several responsibilities related to
    establishing a connection and performing a handshake. Additionally, it handles different types of handshakes (XX, IK) and
    fallback scenarios, leading to increased complexity. To address this smell, I refactored the Connection class by applying
    the Single Responsibility Principle (SRP) and Extract Class technique. Each responsibility (connection management,
    handshake) should ideally be encapsulated within its own class.
    I refactored the code into two separate classes: “HandshakeManager” and “Connection”. The code is now more modular,
    making it easier to understand, test, and maintain.

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.

1 participant