std::rand should be cast to double before division

[AkramIzz]

std::rand returns an integer when divided by RAND_MAX, the result will always be 0.

In Ray Tracing In One Weekend

inline double random_double() {
    // Returns a random real in [0,1).
    return std::rand() / (RAND_MAX + 1.0);
}

Should be (following book style)

inline double random_double() {
    // Returns a random real in [0,1).
    return double(std::rand()) / (RAND_MAX + 1.0);
}

Happy to make the change myself, just following the contribution guide.

PS: Thank you for the great books and all the work you put into this!! Let me know if you have a donate button somewhere (I couldn't find any), would love to buy you a coffee :).

[dimitry-ishenko]

std::rand returns an integer when divided by RAND_MAX, the result will always be 0.

@AkramIzz the rest of the world begs to differ: https://godbolt.org/z/8r4q6b9fY

[hollasch]

You're correct that std::rand() / RAND_MAX will evaluate to integer 0 for all possible return values. But the divisor is RAND_MAX + 1.0, which has type double, which yield a quotient of type double.

[AkramIzz]

Oh my bad, I didn't notice that. I wrote +1 while testing that line instead of +1.0 and copied the line when writing the issue description.

Thanks for clarifying! I don't know what the convention in c++ is but I think making it explicit would be less error prone specially for people less experienced in c/c++ like me.

[hollasch]

Yeah, I get your point. We're constantly wrestling with nuances like this, and are definitely mindful of programmers that aren't fluent in C++. That said, these semantics are fairly common across most languages (implicit promotion to the most expansive type), to the point that I can't really get my head around how it would work otherwise. But I also get your point about this perhaps being too subtle. Sigh.

I think for now I'm going to leave it as-is in the interest of brevity, and because this is what we do in many other places in the code.

[dimitry-ishenko]

I think making it explicit would be less error prone specially for people less experienced in c/c++ like me.

@AkramIzz this is not C/C++ specific. Almost all typed languages with distinct integer and floating point types behave the same way. Click links below and press RUN to see for yourself.

• Java
• C#
• Go
• Python
• Swift

Exceptions:

• Rust requires explicit cast
• JS/TS and LUA store all numbers as floating point

[dimitry-ishenko]

@hollasch IMHO there is no need to sweat over it...

[dimitry-ishenko]

@hollasch it would possibly help if you switched RAND_MAX and 1.0 around:

inline double random_double() {
    // Returns a random real in [0,1).
    return std::rand() / (1.0 + RAND_MAX);
}

This way people don't have to read all the way until the end of the line to see a floating point number. 😏

[AkramIzz]

I'm referring to integer division not the implicit conversion. Yes implicit conversion is almost done in all languages, but integer division is more nuanced.

Python, dart, swift, and rust prevent such thing, in different ways, eg. rust requires explicit conversion, swift doesn't define the operator for mixed type division and someDouble = 1 / 2 will produce 0.5, dart and python have different operators for integer division and floating-point division.. And there is languages that have the same behaviour as c/c++ (go, java, c#).

If this was return std::rand(); then there's nothing to discuss, implicit conversion is not the issue, but with division, I think an explicit conversion is less error prone.

Though I get your points, and as @dimitry-ishenko said it's nothing to sweat about. Thanks for your time.

This way people don't have to read all the way until the end of the line to see a floating point number. 😏

Won't help much, I'll probably stop reading before the + RAND_MAX part :P

[dimitry-ishenko]

Won't help much, I'll probably stop reading before the + RAND_MAX part :P

True that... 😃

@hollasch may it's a good time to switch to something more modern, like:

inline double random_double(double min, double max)
{
    static std::mt19937 gen;
    return std::uniform_real_distribution{min, max}(gen);
}

inline double random_double() { return random_double(0., 1.); }

I think it's even mentioned somewhere in the book (but I am too lazy to look for it right now).