Bug 47962 - Fix string literal bugs and inefficient object creation
Summary: Fix string literal bugs and inefficient object creation
Status: RESOLVED FIXED
Alias: None
Product: POI
Classification: Unclassified
Component: POI Overall (show other bugs)
Version: 3.5-FINAL
Hardware: PC Linux
: P2 normal (vote)
Target Milestone: ---
Assignee: POI Developers List
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-10-08 02:22 UTC by David Horwitz
Modified: 2009-10-08 16:05 UTC (History)
1 user (show)



Attachments
Fix string leteral first and inneficient obejct creation (166.32 KB, patch)
2009-10-08 02:22 UTC, David Horwitz
Details | Diff
Improved patch - please disregard earlier (164.79 KB, patch)
2009-10-08 02:30 UTC, David Horwitz
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Horwitz 2009-10-08 02:22:26 UTC
Created attachment 24361 [details]
Fix string leteral first and inneficient obejct creation

The attached patch fixes 600+ occurrences of 2 common bug patterns identified
by Findbugs and other automated code checkers they are:

1) String literal first bugs (foo.equals("") rather than "".equals(foo))
2) Inefficient object creation (String, Boolean, Integer, Long) - e.g. new
Long(1) rather than Long.valueOf(1)
Comment 1 David Horwitz 2009-10-08 02:30:12 UTC
Created attachment 24362 [details]
Improved patch - please disregard earlier

PLease use second patch the first contains 2 errors
Comment 2 Josh Micich 2009-10-08 16:05:31 UTC
Thanks for the patch - some good clean up work.

Applied in svn r823348 with mods

Not all occurrences of 'foo.equals("bar")' were changed to '"bar".equals(foo)'.  Presumably this change is intended to reduce the likelihood of a NullPointerException.  In many cases however, it was clear that 'foo' could not be null, and the code was left as-is for readability.

Regarding the box constructor vs valueOf() method changes - I'm pretty sure there were no places where POI needs distinct instances, so most of these were OK.  However, a few changes were not applied for these reasons:
  - caching of box instances generally applies to small integral values near zero.
  - Double.valueOf(double) does not utilize a DoubleCache or similar.
  - Boolean.FALSE is better than Boolean.valueOf(false).


If you are keen to fix other compiler warnings that would be very much appreciated.  Please try to keep the patches smaller, to make them easier to QA.