jeromq icon indicating copy to clipboard operation
jeromq copied to clipboard

Refactored few implementation and design code smells.

Open KrishnaVaibhavY opened this issue 2 years ago • 1 comments

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.

KrishnaVaibhavY avatar Nov 28 '23 02:11 KrishnaVaibhavY

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

fbacchella avatar Nov 28 '23 07:11 fbacchella

Also, as per the C4 process, a patch should be a minimal and accurate answer to exactly one identified and agreed problem. This PR implements many fixes to many problems. Due to it's staleness, I'll be closing this issue.

trevorbernard avatar Sep 18 '24 16:09 trevorbernard