[jira] [Commented] (MATH-915) Backward compatibility broken in "EmpiricalDistribution"

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

[jira] [Commented] (MATH-915) Backward compatibility broken in "EmpiricalDistribution"

Walter Laan (Jira)

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

Phil Steitz commented on MATH-915:
----------------------------------

I am sorry I screwed this up in my last commit.  The patch has the right idea, but is not quite correct.  The constructor that takes a RandomGenerator should not be deprecated, but it should call the superclass constructor, which means its null tolerance is going to have to be dropped (which seems to have been broken anyway in 3.0 or before).  What we might consider deprecating are constructors that take RandomDataImpl (incorrectly dropped in my last commit) and RandomDataGenerator (i.e. don't add that at all).  The RandomDataImpl constructors predate RandomGenerator, which is really the underlying source of randomness and the only random data constructor argument really needed.  I will fix this or roll back the last commit so backward compat is restored both here and in ValueServer.

One thing I am not sure I handled best is RandomDataGenerator itself as just a renamed version of RandomDataImpl.  The awkwardness of removing the deprecations makes me think that I did not do a good job designing the replacement - RandomDataGenerator.  This class is new for 3.1, so any comments on how to better accomplish the simple goal of renaming RandomDataImpl would be appreciated.  One thing that I have been experimenting with is having a package-scoped getDelegate() method added to RandomDataImpl.  That will allow RandomDataImpl instances to be replaced by RandomDataGenerators, so there is no need to maintain instances of both.
               

> Backward compatibility broken in "EmpiricalDistribution"
> --------------------------------------------------------
>
>                 Key: MATH-915
>                 URL: https://issues.apache.org/jira/browse/MATH-915
>             Project: Commons Math
>          Issue Type: Bug
>            Reporter: Gilles
>            Priority: Blocker
>             Fix For: 3.1
>
>         Attachments: MATH-915.diff
>
>
> There is a binary-compatibility problem in {{o.a.c.m.random.EmpiricalDistribution}} (cf. "Clirr" report).
> Usage of "RandomDataImpl" has been replaced by "RandomDataGenerator".
> However, unless I'm mistaken, none of those is actually necessary. Moreover, the "randomData" field in this class "shadows" the (deprecated) protected field in the super class. Also, it duplicates functionality (RNG) already present in the super class (through the the "random" protected field).

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira