osdir.com


[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])


Are we instead trying to remove the “extends” from all of those classes?

My thought was to override the method and javadoc locally (in the actual subclass) so that we can give reasonable deprecation messages, but that’s only a thought. I’m ok with whatever.

-Rob

> On Aug 9, 2018, at 7:02 AM, Gilles <gilles@xxxxxxxxxxxxxxxxxxxxx> wrote:
> 
> Hi all.
> 
> On Wed, 08 Aug 2018 19:59:24 +0200, Gilles wrote:
>> Hello Rob.
>> 
>> On Wed, 8 Aug 2018 12:44:16 -0400, Rob Tompkins wrote:
>>> @Gilles - thoughts here?? Just kinda what I was thinking, but I’m
>>> only a +0 on this change. So, if you want to revert it before going up
>>> with 1.1, that’s fine.
>> 
>> I don't understand this change after what I answered to Gary's strange
>> proposal.
>> The comment is *wrong*.  As I said previously, the base class provides
>> boiler-plate code; that is *not* deprecated.  The issue is that those
>> methods were not meant to be used further down the hierarchy (in user's
>> subclasses). [And now, furthermore, they are not used anymore in class
>> "PoissonSampler", following the change in RNG-50 (delegation to other
>> classes).]
>> The sampler classes should have been made "final"; but now, this change
>> would also be "breaking" (even though I doubt about legitimate use-cases
>> for inherithing from the sampler implementations).
> 
> Assuming it's unlikely that application developers would have
> * called protected methods of "SamplerBase",[1]
> * create subclasses of "SamplerBase",[1]
> * create subclasses of the sampler classes,[1]
> I suggest to
> * make "SamplerBase" package private
> * make the sampler clases "final".
> 
> The above are the *less* intrusive fixes, as they would only
> potentially impact application developers by making them aware
> that they have made incorrect usage of an *inadvertently*
> public API.  Correct usage will not be impacted even though
> the change is not BC.[2]
> 
> Any objection to that fix?[3]
> 
> Regards,
> Gilles
> 
> [1] Rationale: (a) There is no reason to do that and
>               (b) "Commons RNG" isn't much used at this point:
>      http://mvnrepository.com/artifact/org.apache.commons/commons-rng-sampling/usages
> [2] I recall that a similar situation (BC breaking in a minor
>    release) occurred in CM, at a time where the number of
>    potential users was much larger.
> [3] I'll add a prominent warning in the changelog to the effect
>    that people wanting to continue with incorrect usage of the
>    samplers should not upgrade. ;-)
> 
>> 
>> Please revert.
>> 
>> Thanks,
>> Gilles
>> 
>>> 
>>> -Rob
>>> 
>>>> On Aug 8, 2018, at 12:42 PM, chtompki@xxxxxxxxxx wrote:
>>>> 
>>>> Repository: commons-rng
>>>> Updated Branches:
>>>> refs/heads/1.1 50b984b1d -> f8159f28a
>>>> 
>>>> 
>>>> Adding PoissonSampler deprecations. Use the correct faster public methods
>>>> 
>>>> 
>>>> Project: http://git-wip-us.apache.org/repos/asf/commons-rng/repo
>>>> Commit: http://git-wip-us.apache.org/repos/asf/commons-rng/commit/f8159f28
>>>> Tree: http://git-wip-us.apache.org/repos/asf/commons-rng/tree/f8159f28
>>>> Diff: http://git-wip-us.apache.org/repos/asf/commons-rng/diff/f8159f28
>>>> 
>>>> Branch: refs/heads/1.1
>>>> Commit: f8159f28a52197d0e7b55e39b115702147cf57a0
>>>> Parents: 50b984b
>>>> Author: Rob Tompkins <chtompki@xxxxxxxxx>
>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>> Committer: Rob Tompkins <chtompki@xxxxxxxxx>
>>>> Committed: Wed Aug 8 12:42:45 2018 -0400
>>>> 
>>>> ----------------------------------------------------------------------
>>>> .../sampling/distribution/PoissonSampler.java   | 42 ++++++++++++++++++++
>>>> 1 file changed, 42 insertions(+)
>>>> ----------------------------------------------------------------------
>>>> 
>>>> 
>>>> http://git-wip-us.apache.org/repos/asf/commons-rng/blob/f8159f28/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> ----------------------------------------------------------------------
>>>> diff --git a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> index d0733ba..29d0e4e 100644
>>>> --- a/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> +++ b/commons-rng-sampling/src/main/java/org/apache/commons/rng/sampling/distribution/PoissonSampler.java
>>>> @@ -67,6 +67,48 @@ public class PoissonSampler
>>>>        return poissonSampler.sample();
>>>>    }
>>>> 
>>>> +    /**
>>>> +     * @return a random value from a uniform distribution in the
>>>> +     * interval {@code [0, 1)}.
>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>> +     * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected double nextDouble() {
>>>> +        return super.nextDouble();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @return a random {@code int} value.
>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>> +     * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected int nextInt() {
>>>> +        return super.nextInt();
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @param max Upper bound (excluded).
>>>> +     * @return a random {@code int} value in the interval {@code [0, max)}.
>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>> +     *      * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected int nextInt(int max) {
>>>> +        return super.nextInt(max);
>>>> +    }
>>>> +
>>>> +    /**
>>>> +     * @return a random {@code long} value.
>>>> +     * @deprecated - one should be using the {@link PoissonSampler#sample()} method,
>>>> +     *      * as it is considerably faster.
>>>> +     */
>>>> +    @Deprecated
>>>> +    protected long nextLong() {
>>>> +        return super.nextLong();
>>>> +    }
>>>> +
>>>>    /** {@inheritDoc} */
>>>>    @Override
>>>>    public String toString() {
>>>> 
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
> For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@xxxxxxxxxxxxxxxxxx
For additional commands, e-mail: dev-help@xxxxxxxxxxxxxxxxxx