jiajunwang commented on a change in pull request #861: Make ClusterSetup
realm-aware
URL: https://github.com/apache/helix/pull/861#discussion_r389074928
##########
File path: helix-core/src/main/java/org/apache/helix/tools/ClusterSetup.java
##########
@@ -1570,4 +1610,66 @@ public static void main(String[] args) throws Exception
{
int ret = processCommandLineArgs(args);
System.exit(ret);
}
+
+ public static class Builder {
Review comment:
For 2) the setter and validation logic should be common for the ZK
parameters. And ZK related input and validation will be evolved the same.
That's why a single class would help us to reduce maintenance costs.
For 1) There are multiple options to avoid the problem. Not sure if we have
these options evaluated. If any document, that would be great so I can take a
look : )
1. Pass the ZK Client builder to the verifier's builders. So there is no
inheritance, but the ZK client building logic is in the client builder only.
2. Use a template class as mentioned here,
https://stackoverflow.com/questions/4031857/way-to-make-java-parent-class-method-return-object-of-child-class
3. Since ZK parameters are in most cases requried, include them in the
constructor and you don't need to worry about the return value. In the verifier
builder, the constructor calls the super(...) constructor to finish the ZK
client build.
There could be more ways.
----------------------------------------------------------------
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]