search

envire / envire-envire_core

Core part for the Environment Representation library
BSD 2-Clause "Simplified" License
7 stars 13 forks source link

Fix random generator creation in Frame ctor #3

Closed arneboe closed 9 years ago

arneboe commented 9 years ago

Before, every time a new Frame was created it also created a new random_generator. Now one random_generator per thread is created and reused.

Also added a very tiny test that ensures that the uuid is generated and not null

saarnold commented 9 years ago

Since the BaseItem has the same issue, what about having a class e.g. envire::random_generator which is a singleton or has a static boost::random_generator and provides the uids?

arneboe commented 9 years ago

I didn't know that BaseItem has the same issue. If more than one class has the issue a global random_generator singleton is the better option. I'll implement it :)

I could limit the scope of the using statement to the class. Would that be ok? Otherwise I'll just remove it.

saarnold commented 9 years ago

Thanks :+1:

I would in general avoid to use the using statement in header files. I think when you use it in the class, the class would appear to have all methods of the included classes, which is maybe also not what you want. And using namespaces makes clearer which type is used and where it comes from. IMHO namespaces make code easier to read and understand.

arneboe commented 9 years ago

Ok I introduced a RandomGenerator.hpp which provides access to a thread local static random_generator instance.

saarnold commented 9 years ago

And please squash your commits to one. We should avoid blurring the commit history with the development process of something that should be one commit. When doing changes in PR's you can also amend your last commit and force push the changes.

Sry to be make this such a pain..

arneboe commented 9 years ago

k, i squashed them and removed the copyright notice. I also added the missing header.

saarnold commented 9 years ago

Ok, thanks.