belliottsmith commented on code in PR #57:
URL: https://github.com/apache/cassandra-accord/pull/57#discussion_r1301991501
##########
accord-core/src/test/java/accord/burn/random/FrequentLargeRange.java:
##########
@@ -18,59 +18,121 @@
package accord.burn.random;
-import accord.utils.Invariants;
+import accord.utils.Gen;
+import accord.utils.Gen.LongGen;
+import accord.utils.Gens;
import accord.utils.RandomSource;
-public class FrequentLargeRange implements RandomLong
+import java.time.Duration;
+import java.util.concurrent.TimeUnit;
+
+public class FrequentLargeRange implements LongGen
{
- private final RandomLong small, large;
- private final double ratio;
- private final int steps;
- private final double lower, upper;
- private int run = -1;
- private long smallCount = 0, largeCount = 0;
-
- public FrequentLargeRange(RandomLong small, RandomLong large, double ratio)
+ private final LongGen small, large;
+ private final Gen<Boolean> runs;
+
+ public FrequentLargeRange(LongGen small, LongGen large, double ratio)
{
- Invariants.checkArgument(ratio > 0 && ratio <= 1);
this.small = small;
this.large = large;
- this.ratio = ratio;
- this.steps = (int) (1 / ratio);
- this.lower = ratio * .8;
- this.upper = ratio * 1.2;
+ this.runs = Gens.bools().biasedRepeatingRuns(ratio);
Review Comment:
I didn't recall seeing any reply to my earlier suggestion, sorry if I missed
it.
Is there a reason some (e.g. `pick`) methods warrant entry to `RandomSource`
but not others?
I think we're going to continue to have friction here. I will continue to
embellish `RandomSource` as this is the natural place for me and how I create
tests, and is why I created the abstraction. You will continue to put
generators into `Gen` because this is how you prefer to write tests, and why
you created that abstraction. This is fine while the approaches do not mix; we
already have plenty of redundancy in test authorship. But we either have to
accept that we have different approaches and stick to the approach already in
use when editing tests, or we have to standardise. I have generally tried to
avoid imposing my views on your approach, as I don't see any reason to debate
abstractions I don't intend to use. But in this case we're editing my work done
with my approach, and so the approaches are clashing, which is why I asked not
to use the `Gen` abstraction. We seem to have as a result slipped into assuming
we need to come to a conclusion about the one true approach, but I fe
ar on this we will ever reach agreement. If we can agree not to impose our
approaches on the other, and when editing tests to try to obey the existing
style (as we do elsewhere in the codebase), I do not see why both (and more!)
approaches, and generators, cannot co-exist.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
