[GitHub] [helix] NealSun96 commented on a change in pull request #1945: Refactor and move ClusterTopologyConfig from nested class to a separate one.

[GitBox <gi...@apache.org>]

NealSun96 commented on a change in pull request #1945:
URL: https://github.com/apache/helix/pull/1945#discussion_r793073771



##########
File path: helix-core/src/main/java/org/apache/helix/model/ClusterConfig.java
##########
@@ -1033,6 +1033,14 @@ public void setAbnormalStateResolverMap(Map<String, String> resolverMap) {
     return idealStateRuleMap;
   }
 
+  /**
+   * Create an instance of {@link ClusterTopologyConfig} based on cluster config.
+   * @return an instance of {@link ClusterTopologyConfig}.
+   */
+  public ClusterTopologyConfig getClusterTopologyConfig() {
+    return ClusterTopologyConfig.fromClusterConfig(this);

Review comment:
       Seems a bit odd to me... Is there a reason why ClusterConfig has a method to construct a ClusterTopologyConfig out of itself? If ClusterTopologyConfig needs to be constructed, the caller should just directly call `ClusterTopologyConfig.fromClusterConfig(clusterConfig)`, instead of `clusterConfig.getClusterTopologyConfig()`; this method doesn't seem that necessary to me. 




-- 
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.

To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org