[Math] MATH-1408 (exceptions for control flow)

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[Math] MATH-1408 (exceptions for control flow)

Gilles Sadowski
Hi.

See:
   https://issues.apache.org/jira/browse/MATH-1408

Any objection to the proposed changes?

Regards,
Gilles


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [Math] MATH-1408 (exceptions for control flow)

Bruno P. Kinoshita-3
TL;DR we are working to replace ClassCastException (CCE) by either an instanceof check, or a different approach. Right now, adding generics to Frequency would fix it, and (I think) be a better design for the class

In Eclipse, I looked for the occurrences of CCE in [math], and the first place I picked to work on was the Frequency class [1]. This class provides a frequency table for users (implemented with a TreeMap) and methods to retrieve the mode, unique count, count, sum of frequencies, etc.

The problem is that the Frequency class has an internal TreeMap, and offers users the ability to store chars, ints, or longs, offering different methods to add values, get cumulative frequencies, etc, for int / long / char.

Because of this design, the SortedMap is created with <Comparable<?>, Long>. The keys, accepting any Comparable<?> (there is addValue(Comparable<?>) method), have to check for CCE to make sure the user passed a key of a type that can be compared with the other keys in the TreeMap. And it does even a workaround for invalid values, returning zero, or ignoring errors. The map values also get boxed to Long's, as this supports all int/char values.

I think we can add generics to the Frequency class, as in this fork: https://github.com/apache/commons-math/compare/master...kinow:MATH-1408-generic-frequency?expand=1

By doing that, we would break binary compatibility (it would be fine to wait for a major release to ship it) but we would avoid runtime errors, remove warning suppression annotations, and also remove the need to throw the MathIllegalArgumentException exception when adding values and for other methods too.

Does anyone have any objections to generifying the Frequency class, or would have a use case for a int/long/char Frequency class?

Cheers
Bruno

[1] https://github.com/apache/commons-math/blob/19e0e29908fef67a0890f6a8513494e9963b2ae0/src/main/java/org/apache/commons/math4/stat/Frequency.java



________________________________
From: Gilles <[hidden email]>
To: Commons Developers List <[hidden email]>
Sent: Saturday, 8 April 2017 1:54 AM
Subject: [Math] MATH-1408 (exceptions for control flow)



Hi.


See:

  https://issues.apache.org/jira/browse/MATH-1408


Any objection to the proposed changes?


Regards,

Gilles



---------------------------------------------------------------------

To unsubscribe, e-mail: [hidden email]

For additional commands, e-mail: [hidden email]

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]