[
https://issues.apache.org/jira/browse/BEANUTILS-267?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Niall Pemberton resolved BEANUTILS-267.
---------------------------------------
Resolution: Fixed
Fix Version/s: 1.8.0
Assignee: Niall Pemberton
Fixed, thanks for the suggestion:
http://svn.apache.org/viewvc?view=rev&revision=498105
> BeanComparator(String, Comparator) should check the comparator for null and
> default to ComparableComparator.getInstance()
> -------------------------------------------------------------------------------------------------------------------------
>
> Key: BEANUTILS-267
> URL: https://issues.apache.org/jira/browse/BEANUTILS-267
> Project: Commons BeanUtils
> Issue Type: Improvement
> Components: Bean-Collections
> Affects Versions: 1.7.0
> Reporter: Jacob Kjome
> Assigned To: Niall Pemberton
> Priority: Minor
> Fix For: 1.8.0
>
>
> The way the BeanComparator(String, Comparator) constructor is implemented is
> inconvenient. I've got code that passes in a comparator. This comparator
> may be null. I assumed that the 2-args constructor would sanely ignore a
> null comparator argument and use a default like ReverseComparator does in
> commons-collections, but alas, no. I have to do the null check before I pass
> it in
> For instance, here's the constructor for ReverseComparator, which takes a
> Comparator argument...
> public ReverseComparator(Comparator comparator) {
> if(comparator != null) {
> this.comparator = comparator;
> } else {
> this.comparator = ComparableComparator.getInstance();
> }
> }
> The null check and provided default is convenient and reasonable.
> Here's the current constructor for BeanComparator that can only end in a
> NullPointerException if provided null comparator....
> public BeanComparator( String property, Comparator comparator ) {
> setProperty( property );
> this.comparator = comparator;
> }
> Why not?....
> public BeanComparator( String property, Comparator comparator ) {
> setProperty( property );
> if(comparator != null) {
> this.comparator = comparator;
> } else {
> this.comparator = ComparableComparator.getInstance();
> }
> }
> The fact that BeanComparator allows itself to be put in a bad state by
> storing a null comparator which it later tries to use with no null check,
> guaranteeing a NullPointerException, probably should be considered a bug.
> However, since it works just fine when provided a non-null comparator, I
> consider this more of an "Improvement" opportunity than a bug, thus the
> reported Issue Type. Hopefully this can be applied to the next release.
> Jake
--
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators:
https://issues.apache.org/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
