Hi.

2020-07-25 21:03 UTC+02:00, Alex Herbert <alex.d.herb...@gmail.com>:
>
>
>> On 25 Jul 2020, at 19:25, Gilles Sadowski <gillese...@gmail.com> wrote:
>>
>> Hello.
>>
>> In the thread[1] from which the below quote is extracted:
>>> Would you be willing to provide a PR that deprecates the relevant APIs
>>> and
>>> points to their equivalent in RNG? It will be more cruft we can trim for
>>> 4.0, whenever that happens.
>> Gary mentions that copy/paste is a "coding horror".  My understanding
>> is that he advises dropping functionality available from "Commons RNG"
>> (or a trivial use of it).
>> Case in point is a bug recently reported.[2]
>
> I am the the one who implemented the copy and paste to fix that bug.

A bug-fix was needed, and, up to yesterday, the *only* way
to fix it was copy/paste.
IIUC Gary's request, the next major version of [Lang] could
stop providing utilities which exist in other components, in
order to reduce "coding horrors".

> Note
> that the alternatives would be:
>
> - to bring in a dependency on RNG

+1
[But I doubt this will be accepted.]

> - to change to using ThreadLocalRandom for the same functionality
>
> I considered it the minimal change to port the algorithm and remain
> dependency free.

As noted above, the problem is not the fix, but the fact that
with bloat increases the bug count.

> However the code can still be fixed with one of the
> alternatives.

The proposed alternative in to drop RandomUtils altogether
in the next major release of [Lang]; and refer users to [RNG].

Do you see any problem?

> My suggestion for RandomUtils would be to remove the use of java.lang.Random
> and use ThreadLocalRandom. This class would support all of the method in
> RandomUtils except:
>
> public static float nextFloat(final float startInclusive, final float
> endExclusive)
>
> The method that had a bug in LANG-1592 is actually provided by the JDK if
> you use ThreadLocalRandom. A switch to this generator could accompany the
> RandomUtils class being marked as deprecated and stated as delegating to
> ThreadLocalRandom. The RandomUtils then becomes a pass through to
> ThreadLocalRandom, for example:
>
> public static double nextDouble(final double startInclusive, final double
> endExclusive) {
>     return ThreadLocalRandom.current().nextDouble(startInclusive,
> endExclusive);
> }

Wrapping a one-liner == code bloat.

>>
>> The change amounts to removing "RandomUtils"[3] and moving all
>> functionality from "RandomStringUtils"[4] (and removing it too) over
>> to "Commons Text" that already defines several similar (perhaps all?)
>> utilities[5] using a more robust approach.
>
> A brief look at RandomStringUtils shows it uses:
>
> o.a.c.lang3.RandomUtils:
> public static int nextInt(final int startInclusive, final int endExclusive)
>
> java.util.Random:
> public int nextInt(int bound)
>
> So would be easy to support after removal of RandomUtils.
>
> The [text] RandomStringGenerator provides the same functionality as
> RandomStringUtils (sampling from a character range). There is a speed
> difference (see partially related RNG-54 [1]). The method in Text is slower
> than that in Lang. But I only looked at generating Hex characters in
> RNG-54.

The final comment was already proposing what we (re)discuss
in this thread, i.e. put algorithms that deal with String/char in
[Text].
I'd be surprised that the RNG part of the algorithm would be
responsible for the performance loss in [Text] vs [Lang].
Any problem fixing [Text] in that respect?

> For ease of maintenance dropping RandomStringUtils in favour of the method
> in [text] seems a good approach. Note that lang has methods for specific
> character sets. This could be added as a feature to the
> RandomStringGenerator.Builder in text so switching from one to the other is
> an easier refactoring task.

Sure.

Regards,
Gilles

>
> Alex
>
> [1]
> https://issues.apache.org/jira/browse/RNG-54?focusedCommentId=16582795&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-16582795
> <https://issues.apache.org/jira/browse/RNG-54?focusedCommentId=16582795&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-16582795>
>
>>
>> Is everyone on the same page?
>>
>> Regards,
>> Gilles
>>
>> [1] https://markmail.org/message/s2o3c57537id37jt
>> [2] https://issues.apache.org/jira/browse/LANG-1592
>> [3]
>> https://gitbox.apache.org/repos/asf?p=commons-lang.git;a=blob;f=src/main/java/org/apache/commons/lang3/RandomUtils.java;h=b1c7f0fb147c586a57cd498734bfb5bc92f19f37;hb=HEAD
>> [4]
>> https://gitbox.apache.org/repos/asf?p=commons-lang.git;a=blob;f=src/main/java/org/apache/commons/lang3/RandomStringUtils.java;h=0d6df4eed0d91bbd32ad378c28400fdf049ee542;hb=HEAD
>> [5]
>> https://gitbox.apache.org/repos/asf?p=commons-text.git;a=blob;f=src/main/java/org/apache/commons/text/RandomStringGenerator.java;h=d8e38e4614dba548ef764225b87626f3a4cda434;hb=HEAD

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

Reply via email to