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

Refactored few implementation and design code smells. #966

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

Conversation

KrishnaVaibhavY
Copy link

Implementation Smells

Extract method
Extracted ‘setupBroadcastChannel’ and ‘broadcastBeacon’ in BroadcastClient, and for better code organization and readability. This resolves the one of the implementation smell in the ‘ZBeacon’ class

Rename method/variable
Renamed handle to datagramChannel and isRunning to running for better clarity.

Decompose conditional
Decomposed the complex conditional in BroadcastServer's run method into shouldProcessMessage. Also introduced beaconData in BroadcastClient's broadcastBeacon for clarity.

Design Smells

Pull-up variable/method
Moved the code field and methods related to it (getErrorCode() and a part of toString()) from ZMQException to UncheckedZMQException. This allows any subclass of UncheckedZMQException to have an error code and the associated methods.

Push-down variable/method

The SimpleCurveAuth inner class has a dependency on ZCertStore.Fingerprinter. We can push down the fingerprinter initialization from the ZAuth constructor to SimpleCurveAuth. This makes SimpleCurveAuth more independent and encapsulates its initialization.
From ZAuth to SimplePlainAuth:

SimplePlainAuth uses properties for passwords. These properties are specific to SimplePlainAuth and can be initialized inside it, rather than being passed from ZAuth.

Replace conditional with polymorphism

ZAuth uses a map auths to handle different authentication mechanisms. This can be improved by using polymorphism. Each Auth implementation (SimpleCurveAuth, SimplePlainAuth, SimpleNullAuth) can override a method like handleAuth(ZapRequest request) to encapsulate the authentication logic.

@fbacchella
Copy link
Contributor

Intersting, but please check the formatting rules of code using mvn compile -Pcheckstyle

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.

None yet

2 participants