[LANG-575] HashCodeBuilder reflectionAppend creates unnecessary copy of excludeFields

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 2.4
• Fix Version/s: 2.5
• Component/s: lang.builder.*

Environment

Sun Java JDK 1.6.0_17

• Assignee: Unassigned
• Reporter: Anthony Whitford
• Created: 22/Dec/09 09:12
• Updated: 25/Feb/10 19:51
• Resolved: 09/Jan/10 11:44

Description

See http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang3/builder/HashCodeBuilder.java?view=markup

Please review the implementation for reflectionAppend (lines 174 to 202)... Specifically, see line 182:

List<String> excludedFieldList = excludeFields != null ? Arrays.asList(excludeFields) : Collections.<String>emptyList();

Note that if you are in the habit of passing in a String array for excluding fields (String[] excludeFields) – which is a best practice when using Hibernate (to skip primary keys (@id) and version fields (@version) that change upon persistence) – EVERY TIME the hashCode is calculated, an ArrayList is being created – generating fodder for the garbage collector.

I thought I might get around this by passing a Collection<String> instead of a String[], but ironically the implementation of the reflectionHashCode(Object object, Collection<String> excludeFields) (see lines 475 to 477), for example, transforms the Collection<String> into a String[] only to have it transformed internally into a temporary ArrayList<String>.

I would expect the implementation to use and read what is submitted, whether that is a String[] or a Collection<String>. I don't think it needs to create another copy just to have a convenient contains method. Efficiency is important, especially in the event of rehashing.