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

Update documentation for ReadRng error handling #326

Merged
merged 1 commit into from
Mar 23, 2018

Conversation

pitdicker
Copy link
Contributor

I will tell you, I hate touching any part of our error handling now 😄. But now it used messages specific for OsRng, while that is exactly where it is not used...

Actually I was going over the differences between your master branch and current master, to see how far we are with porting over the changes. The documentation of ReadRng was one of the small changes missing.

When #319 and #320 land there is not much left. Only three things are left:

  • Range core that can be used in combination with HighPrecision01;
  • Separate traits for Choose and Shuffle;
  • An owned version of WeightedChoice.

The last two could use some more design work, but I think I can make a PR with the first one. It seems to me a separate distribution HighPrecisionRange will do. Then you can also finally track master 🎉.

And provide custom error messages (copied from OsRng before)
@pitdicker pitdicker added the C-docs Documentation label Mar 23, 2018
@dhardy
Copy link
Member

dhardy commented Mar 23, 2018

What was the reason for removing checking for WouldBlock?

A HighPrecisionRange can potentially land in 0.5.1. I don't like the idea of having a completely different range so much though. What do you think of adding a HighPrecision wrapper type then implementing support in Uniform/Range (like the old Open01 wrapper)?

@pitdicker
Copy link
Contributor Author

Because WouldBlock is an error that would be returned when opening a file / device etc. ReadRng works on an already open file, so it should never encounter this error.

@pitdicker
Copy link
Contributor Author

A HighPrecisionRange can potentially land in 0.5.1. I don't like the idea of having a completely different range so much though. What do you think of adding a HighPrecision wrapper type then implementing support in Uniform/Range (like the old Open01 wrapper)?

Yes, I was already planning to say 'feel free to ignore until after 0.5' when opening a PR for that. I thought about a wrapper type also, because a separate implementation of Uniform and Range just for this alternative method does not seem great. But while it can give a little cleaner API, it seems a lot less nice to use.

@dhardy
Copy link
Member

dhardy commented Mar 23, 2018

Okay, then merge if ready.

BTW my 'master' branch has not been updated recently; refer to the 'experimental' branch instead.

I wonder about adding h32/h64 types supporting all the usual ops and .0 to extract the f32/f64 value.

@pitdicker pitdicker merged commit 59f70db into rust-random:master Mar 23, 2018
@pitdicker pitdicker deleted the doc_fixes branch March 23, 2018 09:59
@pitdicker
Copy link
Contributor Author

That 'experimental' branch looks even closer to current master!

pitdicker added a commit that referenced this pull request Apr 4, 2018
Update documentation for ReadRng error handling
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-docs Documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants