Random API proposal #132
Random API proposal #132
Conversation
| fun Random(seed: Long): Random | ||
|
|
||
| // JVM-only: functions to wrap java.util.Random in kotlin.random.Random and vice-versa | ||
| fun java.util.Random.asKotlinRandom(): Random |
There was a problem hiding this comment.
Shouldn't it be .toKotlinRandom() to keep consistency with .to*() functions provided by stdlib for variable types already?
There was a problem hiding this comment.
No, because "to" would suggest that you create a new random instance which is no longer dependent on the original. This only wraps the java random instance.
This is consistent with the naming. One other example of this would be List<T>.asReversed.
There was a problem hiding this comment.
I personally dont understand why we need Random.asJavaRandom() and java.util.Random.asKotlinRandom() on the Random object. Anyone can explain it to me ?
There was a problem hiding this comment.
Imagine you use 2 libraries. One written in java and the other one in Kotlin. Both need a random object and you want to use the same one. Now you have to pass them from one library to the other. The problem now is that the java library expects an instance of type java.util.Random and the kotlin library expects a instance of type kotlin.random.Random, but those 2 are not the same. That's why you need those 2 functions to create wrappers around the objects to "cast" type to the other.
There was a problem hiding this comment.
Thanks for your explanation, it's more clear for me now. But if tomorrow, the implementation of ThreadLocalRandom change in Java, these methods will be a problem no ? (because of multiplatform compatibility constrain)
There was a problem hiding this comment.
There's no multiplatform compatibility constraint for an arbitrary implementation of Random, only for that returned by Random(seed) function.
|
|
||
| open fun nextLong(): Long | ||
| open fun nextLong(bound: Long): Long | ||
| open fun nextLong(origin: Long, bound: Long): Long |
There was a problem hiding this comment.
I find the parameter names origin and bound very non-intuitive. I kinda guess they mean min and max, but I still have no idea if they are inclusive or exclusive. I don't think we should copy obscure parameter names from JDK.
IMHO the most intuitive naming would be the same as for Kotlin ranges: start and endInclusive , or a variation on this like diapasonStart and diapasonEndInclusive
There was a problem hiding this comment.
bound is an exclusive end, something that shouldn't be crossed. This might look unclear in the API summary, but I hope that the method documentation will clarify that. If it won't, we can reconsider the parameter names.
There was a problem hiding this comment.
Ok, then it's diapasonStart and diapasonEndExclusive. You see what I mean?
There was a problem hiding this comment.
origin is really a bit confusing. It may give an impression that this parameter is maybe somehow used as the source of randomness instead of the internal state of the class, or that the returned value is tied to this parameter value (more than to bound) in some other sense.
There was a problem hiding this comment.
I've recorded the question about parameter names.
Ok, then it's diapasonStart and diapasonEndExclusive
While we have other places in the library, where we specify half-open range with two parameters, we haven't used exclusive word in identifiers yet: usually it's just startIndex/endIndex
There was a problem hiding this comment.
We have settled on from and until parameter names.
|
|
||
| open fun nextDouble(): Double | ||
| open fun nextDouble(bound: Double): Double | ||
| open fun nextDouble(origin: Double, bound: Double): Double |
There was a problem hiding this comment.
What was the reason to have 3 overloaded methods instead of one with default parameters like fun nextDouble(origin: Double = 0.0, bound: Double = Double.MAX_VALUE)?
There was a problem hiding this comment.
nextDouble()returns value from 0.0 and up to 1.0- overloads with less parameters allow more efficient implementations (less parameter checking, less edge cases)
There was a problem hiding this comment.
I see. Then 3 questions:
- How much more efficient is
nextDouble()than a method with default parameters? - Is that efficiency worth having a separate
nextDouble()? - Is that efficiency still worth having a relatively rare
nextDouble(bound: Double)?
Note that 2) and 3) combined effectively bloat the API two fold.
There was a problem hiding this comment.
I don't have the precise measurements, but all of the following operations definitely add up. nextDouble without parameters doesn't have to:
- check if the optional parameters are specified and substitute them with their default values
- check if those parameters are valid
- calculate the range
bound - origin - check if it is finite
- multiply the random [0, 1) by the range and add the origin
- check if the resulting value is equal to bound.
|
Given everyone who participated in this discussion, I'm guessing that you'd all be interested in this proposal as well. Add a CSPRNG (eg. SecureRandom) to the Kotlin StdLib #184 I'd love your feedback. |
Discussion goes in KEEP-131
Please don't use this pull request for questions, only for proposals how to improve the text.