Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

guide: random secret number remarks #16354

Closed
vks opened this issue Aug 8, 2014 · 12 comments
Closed

guide: random secret number remarks #16354

vks opened this issue Aug 8, 2014 · 12 comments

Comments

@vks
Copy link
Contributor

vks commented Aug 8, 2014

It might be worth mentioning that generating a uint directly is more efficient than generating an int and calling abs (unless the compiler is very clever).

When reading the code I was wondering why

let secret_number = (rand::random() % 100u) + 1;

would not compile. Shouldn't the compiler be able to infer the correct specialization of random?

Also, it might be more idiomatic to write

let secret_number = (rand::random::<uint>() % 100) + 1;

than using redundant integer literals

let secret_number = (rand::random::<uint>() % 100u) + 1u;
@steveklabnik
Copy link
Member

The first is true, but we never talked about signed vs. unsigned, so I just left it at that, rather than dig into those details.

You can't understand why rand::random can't infer the type without understanding generics, which we don't get to till much later.

The last one is true, but is related to #15526 . until that's resolved, all numbers are suffixed, basically. I haven't actually determined a good guideline for what should be suffixed and what shouldn't, with regards to style.

@tbu-
Copy link
Contributor

tbu- commented Aug 13, 2014

One shouldn't encourage generating random integers like that, this way the they're not equally distributed.

The right way should be:

rand::task_rng().gen_range(1u32, 100+1)

@huonw
Copy link
Member

huonw commented Aug 13, 2014

cc #15954

@tbu- that also requires importing the Rng trait which is beyond what hsa been mentioned up to that point (cc #15729 (comment)).

@tbu-
Copy link
Contributor

tbu- commented Aug 13, 2014

@huonw I think that showing wrong examples is bad.

I'd mention it as "The rand::Rng import ensures that you can use the random number generator, the specifics will be discussed in section '...'".

@vks
Copy link
Contributor Author

vks commented Aug 13, 2014

I think it should at least be mentioned that this approach is not ideal.

@steveklabnik
Copy link
Member

random() is fine except when you really need that 0.00000000000000013553%. I totally support putting this information in random's API docs, but it's a distraction in the Guide.

@thestinger
Copy link
Contributor

Encouraging incorrect code is still not a great thing to do anywhere. It also means people who are aware of the issue are going to put less faith in the guide once they spot it.

@steveklabnik
Copy link
Member

If random() is incorrect, then it should be removed.

@thestinger
Copy link
Contributor

The random function isn't incorrect, the specific usage is.

@steveklabnik
Copy link
Member

Why? We don't need to care that it's not perfectly uniform.

@steveklabnik
Copy link
Member

Phrased another way, if random isn't acceptable here, when IS it acceptable?

@vks
Copy link
Contributor Author

vks commented Aug 16, 2014

It is "acceptable" when you want to generate a random number without restrictions. For int you probably don't want that very often, but for f64 this is usually what you want (note that for floats only the mantissa gets randomized, the exponent is always zero).

I agree that in this use case the minimal bias does not matter.
However, I also think that this approach is not idiomatic, and that this should be mentioned in the guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants