16
OXCE Bugs from OXC / Re: Random generator skewed
« on: October 04, 2023, 07:36:43 am »No, `RNG::generate` work correctly, and changing it would break any other code that use it.
Only correct solution is fix only this usage of random as it should return values `0,1,2` or `1,2,3`.
This mean more correct would be `RNG::generate(1, _totalWeight);` here if `_totalWeight > 0`
Yes, your suggestion would definitely fix it here.
However, from professional experience, a random number function that generates inclusive end-bounds is prone to bugs and is generally more inconvenient to use. This is due to often needing to specify -1 or starting from 1 (0-based indexing is more common in languages, and mixing both leads to bugs). I mentioned a refactor, so we definitely would need to go to each call and fixing it if need be. I wouldn't be surprised if other off-by-one errors weren't also present.
Changing the signature of generate() would bring it in-line with widely-used industry standards.
Example Java - nextInt generates values [0, bound) : https://docs.oracle.com/javase/8/docs/api/java/util/Random.html#nextInt-int-
Example C# (generates exclusive end bound): https://learn.microsoft.com/en-us/dotnet/api/system.random.next?view=net-7.0#system-random-next(system-int32-system-int32
Example Go (same as Java): https://pkg.go.dev/math/rand#Int31n
Also a side note: the generate() function as implemented here actually has a tiny skew even if used properly. It's because it's unlikely that max + 1 - min neatly divides into the range of next(). When you take the %, it slightly biases the values to the lower end of the requested range. However, this bias is so tiny that unless you're doing a very careful statistical analysis or max + 1 - min is huge (like close to 2^30), you'll never notice it.