pkuwm commented on a change in pull request #655: Add a flag for using
dedicated ZkConnection in ZkBaseDataAccessor
URL: https://github.com/apache/helix/pull/655#discussion_r356959073
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ZkBaseDataAccessor.java
##########
@@ -141,6 +157,53 @@ public boolean create(String path, T record, int options)
{
return result._retCode == RetCode.OK;
}
+ /**
+ * Creates a ZkBaseDataAccessor with a custom implementation of ZkSerializer.
+ * If dedicated flag is set to true, it will use a dedicated ZkConnection,
which allows ephemeral
+ * node creation, callbacks, and session management.
+ * If dedicated flag is set to false, it will use a shared ZkConnection,
which only allows simple
+ * CRUD operations to ZooKeeper.
+ * @param zkAddress
+ * @param zkSerializer
+ * @param dedicated
+ */
+ public ZkBaseDataAccessor(String zkAddress, ZkSerializer zkSerializer,
boolean dedicated) {
Review comment:
Just my 2 cents: to me, passing a boolean param to a method to determine the
behavior is kind of code smell.
See
*
https://softwareengineering.stackexchange.com/questions/147977/is-it-wrong-to-use-a-boolean-parameter-to-determine-behavior
* https://martinfowler.com/bliki/FlagArgument.html
Can we have a better design for this? In my opinion, at least, we can create
an enum and pass it:
```
enum ZkClientType {
DedicatedZkClient, SharedZkClient;
}
ZkBaseDataAccessor(String zkAddress, ZkSerializer zkSerializer, ZkClientType
type);
```
I think this would look better than this ugly boolean flag..
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
