> 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. Note that the alternatives would be: - to bring in a dependency on RNG - to change to using ThreadLocalRandom for the same functionality I considered it the minimal change to port the algorithm and remain dependency free. However the code can still be fixed with one of the alternatives. 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); } > > 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. 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. 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 >