[
https://issues.apache.org/jira/browse/MATH-915?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13532325#comment-13532325
]
Gilles commented on MATH-915:
-----------------------------
bq. The constructor that takes a RandomGenerator should not be deprecated
It is not deprecated.
bq. but it should call the superclass constructor
>From what I see, in "EmpiricalDistribution", the source of randomness is
>"RandomDataGenerator".
I indeed thought that it was redundant and first tried removing all reference
to "RandomDataGenerator" in "EmpiricalDistribution". But then it did not work
with "ValueServer" since that class internally uses an instance of
"EmpiricalDistribution" created with a "RandomDataGenerator" argument!
Thus although I would gladly remove all usage of "RandomDataGenerator", it is
not obvious to do it in a clean yet backward-compatible way...
In the meantime, calling the superclass's constructor is not done on purpose:
to avoid that some methods use the superclass's (protected) "random" field
while others use this class's "randomData" (or the superclass's deprecated
"randomData").
bq. What we might consider deprecating are constructors that take RandomDataImpl
This is exactly what I've done in the patch.
bq. any comments on how to better accomplish the simple goal of renaming
RandomDataImpl would be appreciated.
IMO, the problem is not with the renaming, it is having protected fields
(instances of "RandomDataImpl", "RandomDataGenerator", and "RandomGenerator")
which prevent an easy transition path.
For one thing, in 4.0, the "random" field in "AbstractRealDistribution" should
become private.
Until we can start making incompatible changes, I think that the current patch
is probably as good as any other half-baked solution: "Old" usage of
"RandomDataImpl" will be delegated to the new "RandomDataGenerator".
As I said above further cleanup should remove usage of _both_ classes in CM.
IMHO, "RandomDataGenerator" should only be a _user_ tool (syntactic sugar to
bypass direct instantiation of a "distribution" object).
> 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
