[ https://issues.apache.org/jira/browse/MATH-259?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12700183#action_12700183 ]
Sebb commented on MATH-259: --------------------------- See: URL: http://svn.apache.org/viewvc?rev=765996&view=rev Log: MATH-259 - check for Comparable when adding values I overlooked the change of Exception, so there's also: URL: http://svn.apache.org/viewvc?rev=766003&view=rev Log: MATH-259 - throw IllegalArgument rather than ClassCast to better retain original behaviour == I added a new method public void addValue(Comparable<?> v) which is called from public void addValue(Object v) which I took the liberty of deprecating, so the compiler will warn users about non-Comparable objects. Hope that's OK. Note that it's still possible for mutually non-Comparable values to be added, because the code does not check comparisons both ways, it relies on HashMap to do so. I.e. if B.compareTo(A) is OK, but A.compareTo(B) does not exist, then it is possible to add A, then B. This causes a ClassCastException in some of the getXXX() methods. > Bugs in Frequency API > --------------------- > > Key: MATH-259 > URL: https://issues.apache.org/jira/browse/MATH-259 > Project: Commons Math > Issue Type: Bug > Reporter: Sebb > > I think the existing Frequency API has some bugs in it. > The addValue(Object v) method allows one to add a plain Object, but one > cannot add anything further to the instance, as the second add fails with > IllegalArgumentException. > In fact, the problem is with the first call to addValue(Object) which should > not allow a plain Object to be added - it should only allow Comparable > objects. > This could be fixed by checking that the object is Comparable. > Similar considerations apply to the getCumFreq(Object) and getCumPct(Object) > methods - they will only work with objects that implement Comparable. > The getCount(Object) and getPct(Object) methods don't fail when given a > non-Comparable object (because the class cast exception is caught), however > they just return 0 as if the object was not present: > {code} > final Object OBJ = new Object(); > f.addValue(OBJ); // This ought to fail, but doesn't, causing the > unexpected behaviour below > System.out.println(f.getCount(OBJ)); // 0 > System.out.println(f.getPct(OBJ)); // 0.0 > {code} > Rather than adding extra checks for Comparable, it seems to me that the API > would be much improved by using Comparable instead of Object. > Also, it should make it easier to implement generics. > However, this would cause compilation failures for some programs that pass > Object rather than Comparable to the class. > These would need recoding, but I think they would continue to run OK against > the new API. > It would also affect the run-time behaviour slightly, as the first attempt to > add a non-Comparable object would fail, rather than the second add of a > possibly valid object. > But is that a viable program? It can only add one object, and any attempt to > get statistics will either return 0 or an Exception, and applying the > instanceof fix would also cause it to fail. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.