commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Rob Tompkins <chtom...@gmail.com>
Subject Re: [All][RNG] Fix minor design issue (Was: commons-rng git commit: [...] deprecations. [...])
Date Thu, 09 Aug 2018 12:43:34 GMT


> On Aug 9, 2018, at 8:38 AM, Gilles <gilles@harfang.homelinux.org> wrote:
> 
> On Thu, 9 Aug 2018 08:22:32 -0400, Rob Tompkins wrote:
>> Are we instead trying to remove the “extends” from all of those classes?
> 
> I'm not sure what you mean, sorry.
> 
> "SamplerBase" was meant has a utility/shortcut/boiler-plate in order
> to copy the "rng" argument in one place, instead of having a field
> in each sampler class. It is purely internal to this library (in C++,
> inheritance would have been "private”).

Are we trying to remove SamplerBase all together?

> 
>> 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.
> 
> The message would aim at the wrong target: for developers of this
> library, there is no deprecation (the boiler-plate code is there
> to be used); for application developers, the class should not be
> there to be used (hence: package private).

Then, I guess, we should simply state that we want to eventually delete these methods and
they should not be consumed. Yeah? Like I said…I’m up for whatever. I’m just trying
to balance yours and Gary’s points and find a good middle ground that everyone can live
with.

-Rob

> 
> Gilles
> 
>> -Rob
>> 
>>> On Aug 9, 2018, at 7:02 AM, Gilles <gilles@harfang.homelinux.org> 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@apache.org 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@gmail.com>
>>>>>> Authored: Wed Aug 8 12:42:45 2018 -0400
>>>>>> Committer: Rob Tompkins <chtompki@gmail.com>
>>>>>> 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@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
> 


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message