[
https://issues.apache.org/jira/browse/IO-191?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12666079#action_12666079
]
Peter Lawrey commented on IO-191:
---------------------------------
Here are the types of changes
** Making private and package local final if they can be made final.
This improves clarity as it makes it obvious which fields are changed and which
are not. It improves thread safety and to a small degree performance.
** Use character instead of String were the outcome is the same.
For those who typical perform character calculations instead of String
calculations, this may be confusing. However, my guess is that most people
assume a String append as this is the most common usage.
** Fix javadoc links and remove refer to a class which does not exist.
** Note that the use of foreach loops needs to wait until we switch to Java 5.
I assumed that since generics were used that Java 5 was being used. Which
version of Java are you using which supports Generics but not the foreach loop?
** Make methods/fields static if possible.
This improves clarity that this method/field is not dependant on the instance
and has a performance benefit.
** Remove redundant initialisers
This improve clarity as having a redundant initialiser implies its is used for
something when it is not.
** Using Character.toString(char) instead of new Character(char).toString();
This has margin benefit but saves a pointless object.
** Using String.indexOf('*') instead of String.indexof("*").
This improves clarity by stating you are looking for just one character, not a
String. There is also a performance benefit.
** Using System.arraycopy() instead of a manual array copy.
This can be significantly faster.
** Call Thread.currentThread().interrupt() rather than swallowing an interrupt,
or highlighting when it is ignored.
** Reduce duplication when two constructors are almost the same.
> It would be easier to review and apply this patch if it was broken down to
> pieces based on the different types of changes.
If people feel multiple patches is easier to review I can break it down. I
don't imagine multiple patches are easier to apply.
> > Changing single character string literals to character literals in string
> > concatenations.
> The benefit is insignificant and the drawback is added conceptual complexity.
> Also, in expressions where other parts are variables, there is no syntactical
> hint that it's a string concatenation expression instead of an integer sum.
This is a fair comment. If you feel its worth reviewing on a case by case basis
I am happy to do this. Adding a char to a String should be consider a
mysterious hack, but it may be of marginal benefit.
> why are some parts of the expression strings and other characters.
Is this a question you would like me to answer or you are just raising this as
a hypothetical question someone might ask.
>> Clearing an existing collection instead of replacing it with a newly
>> allocated one.
> Again, the benefit is typically insignificant, but as a drawback an immutable
> collection may become mutable. What if some other code is still concurrently
> iterating the collection? Perhaps the static analyzer has taken this into
> account, but will a future programmer that wants to modify the class?
If the field is final, a future programmer will need to consider this or the
program won't compile.
> Possible improvements using static analysis.
> --------------------------------------------
>
> Key: IO-191
> URL: https://issues.apache.org/jira/browse/IO-191
> Project: Commons IO
> Issue Type: Improvement
> Reporter: Peter Lawrey
> Priority: Trivial
> Attachments: commons-io-static-analysis.patch
>
> Original Estimate: 3h
> Remaining Estimate: 3h
>
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
