commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Phil Steitz (JIRA)" <j...@apache.org>
Subject [jira] Commented: (MATH-473) Frequency: new option: NON-sorted
Date Sat, 15 Jan 2011 18:12:46 GMT

    [ https://issues.apache.org/jira/browse/MATH-473?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12982142#action_12982142
] 

Phil Steitz commented on MATH-473:
----------------------------------

Thanks for the patch!

I like the setup, with a few small comments / suggestions for improvement

0) The name "Distribution" clashes with an already defined (actually very similar in the discrete
case) interface and package in Commons Math.  I also think it is not quite the right name.
 I would prefer "FrequencyDistribution."

1) I don't see the rationale for separating the pct from counts.  I would see these both as
natural to include in FrequencyTable and no need to separate AbstractDistribution from AbstractFrequency.
 I don't see the value of the additional layer of abstraction.   Could be I am missing something?
  I would recommend just adding pct to the base interface, renamed as in 0) and replacing
the two abstract base classes with one, named "AbstractFrequencyDistribution."

2) Run checkstyle (mvn site will do this) and fix any errors.  There are, for example, some
now broken javadoc links in the base interface.

3) We need unit tests for the new stuff.

Thanks for adding the fix for MATH-474.   Nice work!

> Frequency: new option: NON-sorted
> ---------------------------------
>
>                 Key: MATH-473
>                 URL: https://issues.apache.org/jira/browse/MATH-473
>             Project: Commons Math
>          Issue Type: Improvement
>    Affects Versions: 1.0, 1.1, 1.2, 2.0, 2.1
>            Reporter: Dan Checkoway
>             Fix For: 3.0
>
>         Attachments: MATH-473.patch
>
>
> I have a request for enhancement on org.apache.commons.math.stat.Frequency.  I would
like to be able to specify that the the backing map NOT be sorted.  Right now it uses TreeMap.
 I would like to have the option of specifying that sorting is not important, and would in
fact hinder performance, and a plain old HashMap should be used instead.
> i.e. constructor such as:
> public Frequency(boolean sorted);
> If sorted is true, use a TreeMap.  If sorted is false, use a HashMap.  Is this feasible?
 I'd be happy to contribute a patch if that would help.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


Mime
View raw message