[ 
https://issues.apache.org/jira/browse/CLOUDSTACK-9165?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15114750#comment-15114750
 ] 

ASF GitHub Bot commented on CLOUDSTACK-9165:
--------------------------------------------

Github user alexandrelimassantana commented on the pull request:

    https://github.com/apache/cloudstack/pull/1246#issuecomment-174398371
  
    Hello, there is also something bugging me with this piece of code which 
appears on the changed files:
    
    ```java
    final String gatewayCidr = guestNetwork.getNetworkCidr() == null ? 
guestNetwork.getCidr() : guestNetwork.getNetworkCidr();
    ```
    
    First, it is repeated code, which could turn into a function in the 
superclass, since VirtualNetworkApplianceManagerImpl and NetworkHelpImpl are 
implementation classes.
    
    Also, you are calling the function getNetworkCidr twice in the event of 
it's return value not being null. It's both confusing and not efficient. You 
could try writing a separate method for the purpose of correctly setting this 
variable, such as:
    
    ```java
    protected String getValidGatewayCidr(){
        String gatewayCidr = guestNetwork.getNetworkCidr();
        return gatewayCidr == null ? guestNetwork.getCidr() : null;
    }
    ```
    
    These changes would make the code more understandable, provided that you 
could write javadocs on these pieces of code.


> unable to use reserved IP range in a network for external VMs
> -------------------------------------------------------------
>
>                 Key: CLOUDSTACK-9165
>                 URL: https://issues.apache.org/jira/browse/CLOUDSTACK-9165
>             Project: CloudStack
>          Issue Type: Bug
>      Security Level: Public(Anyone can view this level - this is the 
> default.) 
>            Reporter: sudharma jain
>




--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to