[GitHub] [commons-graph] kinow edited a comment on pull request #5: [SANDBOX-511] Minor Improvement commons-graph

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

kinow edited a comment on pull request #5:
URL: https://github.com/apache/commons-graph/pull/5#issuecomment-748590807


   > IMO on type ensure that the reference will never change. For the values types it will ensure that the value cannot change. If you check some of the other apache commons modules you can see that they follow the same approach.
   
   Yup, there are many modules in Commons that use `final`. I use it to ensure immutability in some objects. But not everyone writes
   
   ```java
   public void methodA(final param1, final param2) {
     return param1 + param2;
   }
   ```
   
   Some projects may enforce it. But as far as I am aware, we have nothing in Commons, nor in Commons Graph. So reviewers would have to inspect each PR to ensure the code was consistent in using the `final` modifiers.
   
   Otherwise, in a few months/years, the code would have a few methods and local variables without the `final`, and we would need to keep updating the code again, chasing for variables missing the `final` modifier.
   
   I prefer to use it to enforce immutability on the end-user, not on other developers unless necessary (i.e. not for a variable used once in a method, or a parameter in a method that is private or not intended to be extended). Making the reviewing process a bi simpler IMHO. Let's leave it open and see if others prefer to use the `final` modifier everywhere it's possible in Graph (ping @garydgregory ?).
   
   -Bruno


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