You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudstack.apache.org by GitBox <gi...@apache.org> on 2021/01/18 20:29:33 UTC

[GitHub] [cloudstack] RodrigoDLopez opened a new issue #4596: Variable names outside the standard defined by JAVA

RodrigoDLopez opened a new issue #4596:
URL: https://github.com/apache/cloudstack/issues/4596


   <!--
   Verify first that your issue/request is not already reported on GitHub.
   Also test if the latest release and master branch are affected too.
   Always add information AFTER of these HTML comments, but no need to delete the comments.
   -->
   
   ##### ISSUE TYPE
   <!-- Pick one below and delete the rest -->
    * Enhancement Request
   
   ##### COMPONENT NAME
   <!--
   Categorize the issue, e.g. API, VR, VPN, UI, etc.
   -->
   ~~~
   All the cloudstack project
   ~~~
   
   ##### CLOUDSTACK VERSION
   <!--
   New line separated list of affected versions, commit ID for issues on master branch.
   -->
   
   ~~~
   4.16
   ~~~
   
   ##### SUMMARY
   <!-- Explain the problem/feature briefly -->
   Currently cloudstack uses a lot of legacy code with variable names outside the standard defined by JAVA. With this issue, I would like to express my intention to correct this pattern module by module in a way that facilitates the revisions and possible tests that may be required.
   
   An example of normalization that we can perform, would be:
   ```
       private final Properties _properties = new Properties ();
       private final Map <String, Object> _cmdLineProperties = new HashMap <String, Object> ();
       private StorageComponent _storage;
       private BackoffAlgorithm _backoff
   ```
   For:
   ```
       private Properties properties = new Properties ();
       private Map <String, Object> cmdLineProperties = new HashMap <String, Object> ();
       private StorageComponent storage;
       private BackoffAlgorithm backoff;
   ```
   
   Several variables start with an underline, which differs from the standard defined by JAVA (as we can see on JAVA oracle documentation: https://docs.oracle.com/javase/tutorial/java/nutsandbolts/variables.html), many others use the reserved word FINAL without the need, which makes our code more verbose and ends up increasing the time for reading/revising the new codes. The use for final can also create problems for unit testing. 
   We could discuss which standard we will use, I vote for the CamelCase standard since it seems to me to be the standard adopted by the vast majority of our contributors.
   
   ##### EXPECTED RESULTS
   <!-- What did you expect to happen when running the steps above? -->
   
   ~~~
       private Properties properties = new Properties ();
       private Map <String, Object> cmdLineProperties = new HashMap <String, Object> ();
       private StorageComponent storage;
       private BackoffAlgorithm backoff;
   ~~~
   
   ##### ACTUAL RESULTS
   <!-- What actually happened? -->
   
   <!-- Paste verbatim command output between quotes below -->
   ~~~
       private final Properties _properties = new Properties ();
       private final Map <String, Object> _cmdLineProperties = new HashMap <String, Object> ();
       private StorageComponent _storage;
       private BackoffAlgorithm _backoff
   ~~~
   


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



[GitHub] [cloudstack] RodrigoDLopez commented on issue #4596: Variable names outside the standard defined by JAVA

Posted by GitBox <gi...@apache.org>.
RodrigoDLopez commented on issue #4596:
URL: https://github.com/apache/cloudstack/issues/4596#issuecomment-792282785


   > I don't fully agree with the rigorous removal of `final` but good job @RodrigoDLopez . As for `final`, I'd put it in unless, rather that leave it out until...
   
   Can you please tell me why we need to use FINAL keyword in every variable? as far as I know, we just need to use it in variables that we dont wanna change its value during the execution.
   
   ```
   When to use a final variable :
   
   The only difference between a normal variable and a final variable is that we can re-assign value to a normal variable but we cannot change the value of a final variable once assigned. 
   Hence final variables must be used only for the values that we want to remain constant throughout the execution of program.
   ```
   
   In addition, the indiscriminate use of FINAL makes it difficult to create unit tests
   
   I would appreciate it immensely if you could technically explain to me why we use FINAL in this (incorrect) way throughout the project.
   
   
   
   > @RodrigoDLopez we've our checkstyle plugin for this. For all new code I agree your suggestions can be reviewed and enforced. I don't however agree with all the suggestions.
   
   I know about the checkstyle plugin... but I want to apply what we are talking about in all the ACS project, including the old files.
   With what suggestions you do not agree? can you please explain me why?


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



[GitHub] [cloudstack] DaanHoogland commented on issue #4596: Variable names outside the standard defined by JAVA

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #4596:
URL: https://github.com/apache/cloudstack/issues/4596#issuecomment-792630655


   
   > ```
   > When to use a final variable :
   > 
   > The only difference between a normal variable and a final variable is that we can re-assign value to a normal variable but we cannot change the value of a final variable once assigned. 
   > Hence final variables must be used only for the values that we want to remain constant throughout the execution of program.
   > ```
   
   The above states that we must use `final` if we don't want variables to change. And though the `only` might imply that we must *not* use it otherwise, this is false. It is a soicial issue/dethat fensive programming to use final so that people have to make a conscious decision that change is allowed.
   
   > In addition, the indiscriminate use of FINAL makes it difficult to create unit tests
   
   which indeed would be a reason to make vars not final at times.
   
   > I would appreciate it immensely if you could technically explain to me why we use FINAL in this (incorrect) way throughout the project.
   
   as said above; social/defensive. The reasons only becomes technical when mistakes have been made.
   


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



[GitHub] [cloudstack] rhtyd commented on issue #4596: Variable names outside the standard defined by JAVA

Posted by GitBox <gi...@apache.org>.
rhtyd commented on issue #4596:
URL: https://github.com/apache/cloudstack/issues/4596#issuecomment-791907492


   @RodrigoDLopez we've our checkstyle plugin for this. For all new code I agree your suggestions can be reviewed and enforced. I don't however agree with all the suggestions.


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



[GitHub] [cloudstack] DaanHoogland commented on issue #4596: Variable names outside the standard defined by JAVA

Posted by GitBox <gi...@apache.org>.
DaanHoogland commented on issue #4596:
URL: https://github.com/apache/cloudstack/issues/4596#issuecomment-762739720


   I don't fully agree with the rigorous removal of `final` but good job @RodrigoDLopez . As for `final`, I'd put it in unless, rather that leave it out until...


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