commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Peter Lawrey (JIRA)" <>
Subject [jira] Commented: (IO-191) Possible improvements using static analysis.
Date Thu, 22 Jan 2009 07:31:59 GMT


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

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

> Possible improvements using static analysis.
> --------------------------------------------
>                 Key: IO-191
>                 URL:
>             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.

View raw message