Fixed now in CVS.
-----Original Message-----
From: Phil Steitz
Sent: Wed 8/11/2004 7:29 PM
To: Jakarta Commons Users List [EMAIL PROTECTED]
Cc:
Subject: RE: [Math] Frequency : Is int comparable to Integer through default
comparator
I will fix the bug, as mentioned in my earlier post so that the implementation
matches the javadoc (i.e., Integers, ints, Longs, longs are all treated the same,
stored as Long instances in the TreeMap). Otherwise, incomparable values will cause
runtime exceptions as documented. I am -1 to changing the interface or to trying to
in general make incomparable objects comparable (other than the special case above).
Users who are adding incomparable objects to a frequency distribution are abusing the
API, so a runtime exception is appropriate. A checked exception is neither
appropriate nor desirable in this case, IMHO.
The standard use cases for this class will use Strings, Chars or integral
types uniformly. The requirement that the values be comparable is essential to the
API -- without it cumulative counts / percentages make no sense.
Phil
-----Original Message-----
From: Mark R. Diggory [mailto:[EMAIL PROTECTED]
Sent: Wed 8/11/2004 4:48 PM
To: Jakarta Commons Users List
Cc:
Subject: Re: [Math] Frequency : Is int comparable to Integer through
default comparator
Shing Hing Man wrote:
>Mark,
> Thank you for explaining the cause of the
>IllegalArgumentException !
>
>I think there is no need to implement a 'default
>comparator'
>to make objects of type like Integer and Long
>comparable.
>
>
I t would be a useful addition for users who want that functionality
with java.lang.Numbers and Strings etc, I agree it probably doesn't
need
to be a default. It would make a nice utility for the utils section.
>The current version of the class Frequency would meet
>the needs
>of most user.
>
>
>
Well, the issue goes deeper than comparision of Integer and Long, the
thrown exception is "obtuse", its unclear from the
IllegalArgumentException itself why the ClassCastException occurs until
you look at both the implementation of Frequency and its underlying
TreeMap. As such I think it is not user friendly. We can do better than
this. I agree we can keep the existing expected behavior, but I think
the Exception handling should be improved or at least made more
descriptive. The issue has to do with the fact that the methods never
define the IllegalArgumentException as being "thrown" so user will not
get a compilation error if they do not handle it appropriately. In the
end the result would be unexpected behavior (no matter how much is
documented in the javadoc). The users should be forced to handle the
use
case by catching the Exception in thier code. This could be either by
throwing the IllegalArgumentException (probably a bad choice) or by
creating an Exception specifically for this issue and throwing it from
our methods.
>In most situations, your data could be casted to a
>common type.
>
>
The risk with Frequency is that the interface "promotes" heterogeneous
and incomparable object usage in the methods that are implemented. For
instance:
public void addValue(int v) {
addValue(new Long(v));
}
public void addValue(long v) {
addValue(new Long(v));
}
public void addValue(char v) {
addValue(new Character(v));
}
The fact that the user can call addValue(long) or addValue(int) and
then
call addValue(char) creates a situation where the
interface/implementation itself actually promotes the creation of
incomparable objects in the tree. If this is going to be maintained
then
I would argue there should at least be a comparator that handles these
cases.
>If a user wants to add data of different types, or
>even their
>own custom type, he/she could implement their own
>comparator to
>avoid the ClassCastException, hence the
>IllegalArgumentException,
>when retrieving elements from TreeMap.
>
>
I think we can easily improve upon the implementation to manage the
contents of the Tree better. A good Comparator can provide us better functionality,
better Exception handling is important as well.
-Mark
--
Mark R. Diggory
Software Developer
Harvard MIT Data Center
http://www.hmdc.harvard.edu
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
