-
Notifications
You must be signed in to change notification settings - Fork 6
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
added random_device to intarray.cpp #5
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good pr. see notes
random_array[i] = rand() % 2; | ||
try | ||
{ | ||
std::random_device rd; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move std::random device outside of the loop, calling it multiple times is redundant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See here for an example of it being used. Otp we discussed and I read up some more on it.
std::random_device can fail
- On construction, ie
std::random_device [name]
, throwing an exception and exiting the whole program if left uncaught. - Or when requesting a random number, ie
rd() % 2
. Currently your code would catch this and usesrand()
in the same spot.
I think you have 2 options here. One is the easy way out and the other is more proper/more involved, but whichever you want to do is fine with me:
- For our purposes it's fine if the whole program exits with an exception if
std::random_device
fails. - Possibly, a better way to construct this is to have 2 separate classes, one
IntArraySrand
andIntArrayRandDevice
or something like that, and in a parent classIntArray
attempt to build the array usingIntArrayRandDevice
and if that fails, build it usingIntArraySrand
. This way you could havestd::random_device
contained within theIntArrayRandDevice
class and so if it fails when constructing the class you know you can't use it anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The second option using the classes would also allow for easier reuse later; notably, being able to combine the generators in different ways for different purposes that appeal better to the different rngs' strengths.
{ | ||
std::random_device rd; | ||
random_array[i] = rd() % 2; | ||
} catch(...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try catch rather than actually checking for errors? EDIT: I actually like the try catch. See above.
random_array[i] = rand() % 2; | ||
try | ||
{ | ||
std::random_device rd; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See here for an example of it being used. Otp we discussed and I read up some more on it.
std::random_device can fail
- On construction, ie
std::random_device [name]
, throwing an exception and exiting the whole program if left uncaught. - Or when requesting a random number, ie
rd() % 2
. Currently your code would catch this and usesrand()
in the same spot.
I think you have 2 options here. One is the easy way out and the other is more proper/more involved, but whichever you want to do is fine with me:
- For our purposes it's fine if the whole program exits with an exception if
std::random_device
fails. - Possibly, a better way to construct this is to have 2 separate classes, one
IntArraySrand
andIntArrayRandDevice
or something like that, and in a parent classIntArray
attempt to build the array usingIntArrayRandDevice
and if that fails, build it usingIntArraySrand
. This way you could havestd::random_device
contained within theIntArrayRandDevice
class and so if it fails when constructing the class you know you can't use it anymore.
No description provided.