You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/01/18 09:32:59 UTC
[GitHub] [commons-rng] arturobernalg opened a new pull request #81: Minor Improvement:
arturobernalg opened a new pull request #81:
URL: https://github.com/apache/commons-rng/pull/81
* Using final parameters
* Using final local variables
* Using final for instance variable
Intend to make the code more simple and read-only using final
----------------------------------------------------------------
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] [commons-rng] coveralls commented on pull request #81: Minor Improvement:
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #81:
URL: https://github.com/apache/commons-rng/pull/81#issuecomment-762227569
[](https://coveralls.io/builds/36401313)
Coverage remained the same at 99.508% when pulling **cc8cd5cc0397080ecebedd12129384327cf20a79 on arturobernalg:feature/feature/minor_improvement2** into **b9f778cbf25dc4f21fb6454392424ccf7276cdeb on apache:master**.
----------------------------------------------------------------
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] [commons-rng] garydgregory commented on pull request #81: Minor Improvement:
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #81:
URL: https://github.com/apache/commons-rng/pull/81#issuecomment-762273653
Each component has it's own rule but in general, I prefer to see "final" so
I don't have to think about looking for a reassignment when I look at a
longer method. See
https://garygregory.wordpress.com/2013/01/26/the-final-kiss-in-java/
Gary
On Mon, Jan 18, 2021, 08:28 Alex Herbert <no...@github.com> wrote:
> The change to use final method arguments is a style change. We already use
> code style checks using the PMD rule AvoidReassigningParameters to ensure
> method arguments are effectively final. The gratuitous use of final does
> not add functionality to the code and in keeping with the source code for
> the JDK we have chosen not to add final in front of everything in the
> source.
>
> Also note that using an IDE to refactor all the code to add final has now
> broken code style formatting (see checkstyle failures on Travis) due to
> line length changes.
>
> I would remove all the use of final from all method arguments as this is
> already enforced by build checks. Then it is easy to see in the PR what is
> left to make final as local or instance variables.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/commons-rng/pull/81#issuecomment-762250958>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAJB6N67UEV67NBHC4JFMTLS2QZRBANCNFSM4WHAJJOA>
> .
>
----------------------------------------------------------------
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] [commons-rng] aherbert commented on pull request #81: Minor Improvement:
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #81:
URL: https://github.com/apache/commons-rng/pull/81#issuecomment-762250958
The change to use final method arguments is a style change. We already use code style checks using the PMD rule `AvoidReassigningParameters` to ensure method arguments are effectively final. The gratuitous use of final does not add functionality to the code and in keeping with the source code for the JDK we have chosen not to add final in front of everything in the source.
Also note that using an IDE to refactor all the code to add final has now broken code style formatting (see checkstyle failures on Travis) due to line length changes.
I would remove all the use of final from all method arguments as this is already enforced by build checks. Then it is easy to see in the PR what is left to make final as local or instance variables.
----------------------------------------------------------------
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] [commons-rng] arturobernalg closed pull request #81: Minor Improvement:
Posted by GitBox <gi...@apache.org>.
arturobernalg closed pull request #81:
URL: https://github.com/apache/commons-rng/pull/81
--
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] [commons-rng] aherbert commented on pull request #81: Minor Improvement:
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #81:
URL: https://github.com/apache/commons-rng/pull/81#issuecomment-762378135
Since we use automated checking tools to ensure that parameters are effectively final then I see the use of the final keyword as noise. This is very true for short methods which make up the majority of this code base. I tend to agree with the user who posted on your KISS article as a counter argument for using final. This should be raised as a discussion and/or vote on the dev mailing list before committing.
----------------------------------------------------------------
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] [commons-rng] arturobernalg closed pull request #81: Minor Improvement:
Posted by GitBox <gi...@apache.org>.
arturobernalg closed pull request #81:
URL: https://github.com/apache/commons-rng/pull/81
--
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