You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I do not support this. Calling thread_rng has low overhead (just creating another reference) and could be optimised to just a check and pointer copy. The ThreadRng type is just a handle to another object; unfortunately this is not obvious from the name so ThreadRng::new sounds like it is doing a lot more work than merely creating a handle.
OsRng is also just a handle really and I did think about replacing its new with e.g. os_rng, but didn't see much point.
I don't share that sentiment, new() is just the constructor-by-convention. It is often very lightweight, though not necessarily so. To me it seems like the constructor for ThreadRng (thread_rng) has a non-idiomatic name.
ThreadRng::new sounds like it is doing a lot more work than merely creating a handle.
It is doing a lot of work if it is the first call in the thread.
I agree that we shouldn't simply have thread_rng() -> impl Rng because this prevents users from embedding the type in structs, (which may be useful even though it isn't common usage); hence ThreadRng::new() does make intuitive sense, so this may be a good option.
See also this point about replacing ThreadRng with perhaps two alternatives: StrongRng and WeakRng.
In any case, I think we should postpone these ideas until after the 0.5 release.
Since there do appear to be quite a few people who watch this repository and this is mostly style, can I get some more opinions on this, i.e. do you prefer writing thread_rng() or ThreadRng::new()? We could also have both, but it is redundant.
Either way, we already have the ThreadRng type (fn thread_rng() -> ThreadRng).
Activity
dhardy commentedon Apr 17, 2018
I do not support this. Calling
thread_rnghas low overhead (just creating another reference) and could be optimised to just a check and pointer copy. TheThreadRngtype is just a handle to another object; unfortunately this is not obvious from the name soThreadRng::newsounds like it is doing a lot more work than merely creating a handle.OsRngis also just a handle really and I did think about replacing itsnewwith e.g.os_rng, but didn't see much point.vks commentedon Apr 17, 2018
I don't share that sentiment,
new()is just the constructor-by-convention. It is often very lightweight, though not necessarily so. To me it seems like the constructor forThreadRng(thread_rng) has a non-idiomatic name.It is doing a lot of work if it is the first call in the thread.
That is my point, right now we are inconsistent.
pitdicker commentedon Apr 17, 2018
I wouldn't remove
thread_rng(), it is used too much. But addingThreadRng::new()seems harmless to me.dhardy commentedon Apr 17, 2018
Having both will just lead to pointless style wars.
dhardy commentedon May 3, 2018
I agree that we shouldn't simply have
thread_rng() -> impl Rngbecause this prevents users from embedding the type in structs, (which may be useful even though it isn't common usage); henceThreadRng::new()does make intuitive sense, so this may be a good option.See also this point about replacing
ThreadRngwith perhaps two alternatives:StrongRngandWeakRng.In any case, I think we should postpone these ideas until after the 0.5 release.
dhardy commentedon Jun 15, 2018
Since there do appear to be quite a few people who watch this repository and this is mostly style, can I get some more opinions on this, i.e. do you prefer writing
thread_rng()orThreadRng::new()? We could also have both, but it is redundant.Either way, we already have the
ThreadRngtype (fn thread_rng() -> ThreadRng).dhardy commentedon Aug 1, 2018
There is almost no interest on this, so I guess we can just forget it.