cloudstack-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From alexandrelimassantana <...@git.apache.org>
Subject [GitHub] cloudstack pull request: CLOUDSTACK-9251: Fix issue in scale VM to...
Date Fri, 29 Jan 2016 16:34:34 GMT
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message