[jira] [Updated] (LANG-1179) Add Properties support to StrLookup
[
https://issues.apache.org/jira/browse/LANG-1179?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anthony Whitford updated LANG-1179:
---
Attachment: StrLookup.patch
Revised patch with unit tests.
> Add Properties support to StrLookup
> ---
>
> Key: LANG-1179
> URL: https://issues.apache.org/jira/browse/LANG-1179
> Project: Commons Lang
> Issue Type: Improvement
> Components: lang.text.*
>Affects Versions: 3.4
>Reporter: Anthony Whitford
>Priority: Minor
> Attachments: StrLookup.patch, StrLookup.patch
>
>
> Sadly,
> [Properties|http://docs.oracle.com/javase/7/docs/api/java/util/Properties.html]
> implements {{Map}} instead of {{Map}}, so one
> can not use the {{MapStrLookup}}.
> I propose adding a Properties variant. Patch attached.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Created] (LANG-1179) Add Properties support to StrLookup
Anthony Whitford created LANG-1179:
--
Summary: Add Properties support to StrLookup
Key: LANG-1179
URL: https://issues.apache.org/jira/browse/LANG-1179
Project: Commons Lang
Issue Type: Improvement
Components: lang.text.*
Affects Versions: 3.4
Reporter: Anthony Whitford
Priority: Minor
Attachments: StrLookup.patch
Sadly,
[Properties|http://docs.oracle.com/javase/7/docs/api/java/util/Properties.html]
implements {{Map}} instead of {{Map}}, so one can
not use the {{MapStrLookup}}.
I propose adding a Properties variant. Patch attached.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Updated] (LANG-1179) Add Properties support to StrLookup
[
https://issues.apache.org/jira/browse/LANG-1179?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Anthony Whitford updated LANG-1179:
---
Attachment: StrLookup.patch
> Add Properties support to StrLookup
> ---
>
> Key: LANG-1179
> URL: https://issues.apache.org/jira/browse/LANG-1179
> Project: Commons Lang
> Issue Type: Improvement
> Components: lang.text.*
>Affects Versions: 3.4
>Reporter: Anthony Whitford
>Priority: Minor
> Attachments: StrLookup.patch
>
>
> Sadly,
> [Properties|http://docs.oracle.com/javase/7/docs/api/java/util/Properties.html]
> implements {{Map}} instead of {{Map}}, so one
> can not use the {{MapStrLookup}}.
> I propose adding a Properties variant. Patch attached.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (LANG-745) StrLookup API confusing
[ https://issues.apache.org/jira/browse/LANG-745?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14977899#comment-14977899 ] Anthony Whitford commented on LANG-745: --- +1... The is confusing. > StrLookup API confusing > --- > > Key: LANG-745 > URL: https://issues.apache.org/jira/browse/LANG-745 > Project: Commons Lang > Issue Type: Bug > Components: lang.* >Affects Versions: 3.0 >Reporter: Etienne Neveu > Fix For: Discussion > > > [bayard: copying this from LANG-564] > I don't see the point of having a generic type parameter on the StrLookup > class, if it's not used anywhere. No method / field in StrLookup references > this type parameter. IntelliJ IDEA itself reports a warning: "Type parameter > 'V' is never used". Moreover, Java generics are not reified, so there is no > reliable way to access the type parameter at runtime (and I don't see the > point of doing that anyway...). > While the Javadoc tries to clarify the purpose of a StrLookup, the unused > type parameter is still confusing, and the client code has to un-necessarily > specify type parameters. For example, I have to write: > StrLookup lookup = StrLookup.noneLookup(); > StrLookup lookup2 = StrLookup.systemPropertiesLookup(); > StrLookup lookup3 = StrLookup.mapLookup(integerMap); > instead of > StrLookup lookup = StrLookup.noneLookup(); > StrLookup lookup2 = StrLookup.systemPropertiesLookup(); > StrLookup lookup3 = StrLookup.mapLookup(integerMap); > My best guess is that this type parameter was added when commons-lang was > generified, because StringLookup.mapLookup() takes a generified Map. Doing > this is not really needed, though: we could remove the type parameter > everywhere, and replace the StrLookup.mapLookup()'s Map with a > Map (which is the same as Map, but > shorter). > I guess it's too late to change this now, due to backward compatibility... > But I thought I'd comment just in case it's still possible. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Updated] (POOL-307) GenericKeyedObjectPool - inefficient use of keySet instead of entrySet
[ https://issues.apache.org/jira/browse/POOL-307?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Anthony Whitford updated POOL-307: -- Attachment: GenericKeyedObjectPool.patch > GenericKeyedObjectPool - inefficient use of keySet instead of entrySet > -- > > Key: POOL-307 > URL: https://issues.apache.org/jira/browse/POOL-307 > Project: Commons Pool > Issue Type: Improvement >Affects Versions: 2.4.2 >Reporter: Anthony Whitford >Priority: Trivial > Attachments: GenericKeyedObjectPool.patch > > > Noticed that GenericKeyedObjectPool has several cases of [Inefficient use of > keySet iterator instead of entrySet > iterator|http://findbugs.sourceforge.net/bugDescriptions.html#WMI_WRONG_MAP_ITERATOR]. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (POOL-307) GenericKeyedObjectPool - inefficient use of keySet instead of entrySet
Anthony Whitford created POOL-307: - Summary: GenericKeyedObjectPool - inefficient use of keySet instead of entrySet Key: POOL-307 URL: https://issues.apache.org/jira/browse/POOL-307 Project: Commons Pool Issue Type: Improvement Affects Versions: 2.4.2 Reporter: Anthony Whitford Priority: Trivial Attachments: GenericKeyedObjectPool.patch Noticed that GenericKeyedObjectPool has several cases of [Inefficient use of keySet iterator instead of entrySet iterator|http://findbugs.sourceforge.net/bugDescriptions.html#WMI_WRONG_MAP_ITERATOR]. -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Created] (POOL-306) BaseGenericObjectPool IdentityWrapper equals violates Java spec
Anthony Whitford created POOL-306:
-
Summary: BaseGenericObjectPool IdentityWrapper equals violates
Java spec
Key: POOL-306
URL: https://issues.apache.org/jira/browse/POOL-306
Project: Commons Pool
Issue Type: Bug
Affects Versions: 2.4.2
Reporter: Anthony Whitford
Priority: Minor
Noticed that the {{equals}} implementation assumes the {{other}} parameter can
be successfully cast to {{IdentityWrapper}}:
{code}
@Override
@SuppressWarnings("rawtypes")
public boolean equals(Object other) {
return ((IdentityWrapper) other).instance == instance;
}
{code}
(There is a chance that this could throw a {{ClassCastException}}.)
See
[BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS|http://findbugs.sourceforge.net/bugDescriptions.html#BC_EQUALS_METHOD_SHOULD_WORK_FOR_ALL_OBJECTS].
I recommend:
{code}
@Override
@SuppressWarnings("rawtypes")
public boolean equals(Object other) {
return other instanceof IdentityWrapper && ((IdentityWrapper)
other).instance == instance;
}
{code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] Created: (LANG-657) Add defaultIfBlank similar to defaultIfEmpty to StringUtils
Add defaultIfBlank similar to defaultIfEmpty to StringUtils
---
Key: LANG-657
URL: https://issues.apache.org/jira/browse/LANG-657
Project: Commons Lang
Issue Type: New Feature
Components: lang.*
Affects Versions: 2.5
Environment: n/a
Reporter: Anthony Whitford
Priority: Minor
Similar to
[StringUtils.defaultIfEmpty|http://commons.apache.org/lang/api-3.0-beta/org/apache/commons/lang3/StringUtils.html#defaultIfEmpty(T,
T)], I'd like to also see a {{defaultIfBlank}} method:
{code}
public static String defaultIfBlank (final String str, final String defaultStr)
{
return StringUtils.isBlank(str) ? defaultStr : str;
}
{code}
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-574) HashCodeBuilder append(Object) could be more efficient
[
https://issues.apache.org/jira/browse/LANG-574?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793564#action_12793564
]
Anthony Whitford commented on LANG-574:
---
Note that *CompareToBuilder* also does the *isArray* check (see line 456):
http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang3/builder/CompareToBuilder.java?view=markup
Note that if the _Class_ object was saved to a variable, you could replace:
{code}if (x instanceof long[]){code}
with
{code}if (xclass == long[].class){code}
> HashCodeBuilder append(Object) could be more efficient
> --
>
> Key: LANG-574
> URL: https://issues.apache.org/jira/browse/LANG-574
> Project: Commons Lang
> Issue Type: Improvement
> Components: lang.builder.*
>Affects Versions: 2.4
> Environment: Sun JDK 1.6.0_17
>Reporter: Anthony Whitford
>Priority: Minor
>
> See:
> http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang3/builder/HashCodeBuilder.java?view=markup
> If you use *reflectionHashCode*, you will eventually call
> *reflectionAppend(Object)* for every field (see lines 859 to 890). There is
> a long list of _instanceOf_ checks for the various types of arrays. My
> concern is that having an array field is relatively rare, yet the code does 9
> if/else checks before computing the _hashCode_ for _one_ element. I am
> thinking that it would make a lot of sense to wrap those if/else checks for
> the various array types in an *isArray* check (see Class.isArray).
> If you look at the JDK source code for *Arrays.deepToString*, you will see
> that it checks if the class is an array before checking the various types.
> Note that since it gets the Class, it doesn't need to execute _instanceOf_
> which is likely more expensive...
> I have not done profiling, but I would not be surprised if short circuiting
> these checks could improve performance by an order of magnitude. The
> assumption is that getting the Class of an Object and asking it if it is an
> array is cheaper than asking _instanceOf_ 9 times.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-575) HashCodeBuilder reflectionAppend creates unnecessary copy of excludeFields
[
https://issues.apache.org/jira/browse/LANG-575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793561#action_12793561
]
Anthony Whitford commented on LANG-575:
---
Considering how repetitive this issue is, you may want to consider LANG-283
because there does seem to be a refactoring opportunity here.
> HashCodeBuilder reflectionAppend creates unnecessary copy of excludeFields
> --
>
> Key: LANG-575
> URL: https://issues.apache.org/jira/browse/LANG-575
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.builder.*
>Affects Versions: 2.4
> Environment: Sun Java JDK 1.6.0_17
>Reporter: Anthony Whitford
>
> 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:
> {code}
> List excludedFieldList = excludeFields != null ?
> Arrays.asList(excludeFields) : Collections.emptyList();
> {code}
> 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}} instead
> of a {{String[]}}, but ironically the implementation of the
> {{reflectionHashCode(Object object, Collection excludeFields)}} (see
> lines 475 to 477), for example, transforms the {{Collection}} into a
> {{String[]}} only to have it transformed internally into a temporary
> {{ArrayList}}.
> I would expect the implementation to use and read what is submitted, whether
> that is a {{String[]}} or a {{Collection}}. 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.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Commented: (LANG-575) HashCodeBuilder reflectionAppend creates unnecessary copy of excludeFields
[
https://issues.apache.org/jira/browse/LANG-575?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12793560#action_12793560
]
Anthony Whitford commented on LANG-575:
---
Note that *EqualsBuilder* has the exact same issue.
See:
http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang3/builder/EqualsBuilder.java?view=markup
*reflectionAppend*, line 321:
{code}
List excludedFieldList = excludeFields != null ?
Arrays.asList(excludeFields) : Collections.emptyList();
{code}
And so does *CompareToBuilder*...
See:
http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang3/builder/CompareToBuilder.java?view=markup
*reflectionAppend*, line 356:
{code}
List excludedFieldList = excludeFields != null ?
Arrays.asList(excludeFields) : Collections.emptyList();
{code}
> HashCodeBuilder reflectionAppend creates unnecessary copy of excludeFields
> --
>
> Key: LANG-575
> URL: https://issues.apache.org/jira/browse/LANG-575
> Project: Commons Lang
> Issue Type: Bug
> Components: lang.builder.*
>Affects Versions: 2.4
> Environment: Sun Java JDK 1.6.0_17
>Reporter: Anthony Whitford
>
> 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:
> {code}
> List excludedFieldList = excludeFields != null ?
> Arrays.asList(excludeFields) : Collections.emptyList();
> {code}
> 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}} instead
> of a {{String[]}}, but ironically the implementation of the
> {{reflectionHashCode(Object object, Collection excludeFields)}} (see
> lines 475 to 477), for example, transforms the {{Collection}} into a
> {{String[]}} only to have it transformed internally into a temporary
> {{ArrayList}}.
> I would expect the implementation to use and read what is submitted, whether
> that is a {{String[]}} or a {{Collection}}. 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.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Created: (LANG-575) HashCodeBuilder reflectionAppend creates unnecessary copy of excludeFields
HashCodeBuilder reflectionAppend creates unnecessary copy of excludeFields
--
Key: LANG-575
URL: https://issues.apache.org/jira/browse/LANG-575
Project: Commons Lang
Issue Type: Bug
Components: lang.builder.*
Affects Versions: 2.4
Environment: Sun Java JDK 1.6.0_17
Reporter: Anthony Whitford
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:
{code}
List excludedFieldList = excludeFields != null ?
Arrays.asList(excludeFields) : Collections.emptyList();
{code}
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}} instead
of a {{String[]}}, but ironically the implementation of the
{{reflectionHashCode(Object object, Collection excludeFields)}} (see
lines 475 to 477), for example, transforms the {{Collection}} into a
{{String[]}} only to have it transformed internally into a temporary
{{ArrayList}}.
I would expect the implementation to use and read what is submitted, whether
that is a {{String[]}} or a {{Collection}}. 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.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
[jira] Created: (LANG-574) HashCodeBuilder append(Object) could be more efficient
HashCodeBuilder append(Object) could be more efficient -- Key: LANG-574 URL: https://issues.apache.org/jira/browse/LANG-574 Project: Commons Lang Issue Type: Improvement Components: lang.builder.* Affects Versions: 2.4 Environment: Sun JDK 1.6.0_17 Reporter: Anthony Whitford Priority: Minor See: http://svn.apache.org/viewvc/commons/proper/lang/trunk/src/java/org/apache/commons/lang3/builder/HashCodeBuilder.java?view=markup If you use *reflectionHashCode*, you will eventually call *reflectionAppend(Object)* for every field (see lines 859 to 890). There is a long list of _instanceOf_ checks for the various types of arrays. My concern is that having an array field is relatively rare, yet the code does 9 if/else checks before computing the _hashCode_ for _one_ element. I am thinking that it would make a lot of sense to wrap those if/else checks for the various array types in an *isArray* check (see Class.isArray). If you look at the JDK source code for *Arrays.deepToString*, you will see that it checks if the class is an array before checking the various types. Note that since it gets the Class, it doesn't need to execute _instanceOf_ which is likely more expensive... I have not done profiling, but I would not be surprised if short circuiting these checks could improve performance by an order of magnitude. The assumption is that getting the Class of an Object and asking it if it is an array is cheaper than asking _instanceOf_ 9 times. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.
