Github user alexandrelimassantana commented on the pull request:

    https://github.com/apache/cloudstack/pull/1363#issuecomment-176851830
  
    @ustcweizhou there are some modifications you could do to make this code a 
little more clean and readable.
    
    **1)** You are duplicating code, since two classes have the same code 
written. You should reallocate this method to a common superclass.
    
    **2)** You could change lines **84-86** (ScaleVMCmd.java file) from line 
comments to the Javadoc's pattern, which can be done much like a block comment, 
but with double asterisks /** Javadoc **/
    
    ```Java
    /**
    * I am a Javadoc and this is general information about the method
    * @param This is a param documentation.
    * @return This is a method's return documentation.
    **/
    ```
    
    **3)** You should use **!details.isEmpty()** instead of **details.size() != 
0**. It better states what you are looking for and it's performance is slightly 
better than the comparison you are using.
    
    **4)** It's a good practice to declare the variables as the first 
statements of the method, that will help when someone is reading inside 
conditionals, the same method written with these changes would look like:
    
    ```Java
    public Map<String, String> getDetails(){
        Map<String, String> customparameterMap = new HashMap<String, String>();
        Collection parameterCollection;
        Iterator iter;
        HashMap<String, String> value;
    
        if (details == null || details.isEmpty())
            return customparameterMap;
    
        parameterCollection = details.values();
        iter = parameterCollection.iterator();
    
        while (iter.hasNext()) {
            value = (HashMap<String, String>)iter.next();
            for (String key : value.keySet())
                customparameterMap.put(key, value.get(key));
        }
    
        return customparameterMap;
    }
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

Reply via email to