cassandra-pr mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [cassandra] yifan-c commented on a change in pull request #388: CASSANDRA-15429: Support NodeTool in dtest
Date Tue, 10 Dec 2019 21:58:59 GMT
yifan-c commented on a change in pull request #388: CASSANDRA-15429: Support NodeTool in dtest
URL: https://github.com/apache/cassandra/pull/388#discussion_r356300575
 
 

 ##########
 File path: src/java/org/apache/cassandra/tools/NodeTool.java
 ##########
 @@ -46,7 +47,20 @@
 {
     private static final String HISTORYFILE = "nodetool.history";
 
+    private static NodeProbeFactory nodeProbeFactory = new NodeProbeFactory();
 
 Review comment:
   The `NodeToolCmd` depends on `NodeProbe`. In order to inject the mock dependency, we either
passing the mock instance to the cmd instances or like what the patch current have, depends
on a static field. Why the `nodeProbeFactory` cannot be an instance field? Because the `NodeToolCmd`
is a static inner class, it cannot see unless the field is a class field. 
   
   If passing the mock instance reference, we will end up having the following code in the
`NodeTool#execute` method, which I think is ugly. The reason of injecting dependency by doing
the type check and casting is that cli parses command into runnable. 
   
   ```
               Runnable parse = parser.parse(args);
               if (parse instanceof NodeToolCmd) // hmmm....
               {
                   ((NodeToolCmd) parse).setNodeProbeFactory(nodeProbeFactory); // or pass
in the probe reference
               }
               printHistory(args);
               parse.run();
   ``` 

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org


Mime
View raw message