Is this the OK scoped way to get random element fr

Question

recently I decided to try out new c++11 random lib, and one thing came to mind... to get rid of the [rand()%nelemennts] when picking random element from container. Reason is that i want repeatable generation, when using rand encapsulation is nonexistant because

auto set_a=generateSet(nelements1); //generateSet calls rand
auto set_b=generateSet(nelements2); //so set_b is determined by the previous line :(

So this is what I came up with: (note that this isnt thread safe, it is designed to be safe in a way that calls to generateSet dont affect eachother(through changing state of rand internal value))

template<typename container_type,typename element_type >
class RandElemGetter
{

    const container_type& containter_ref;
    std::uniform_int_distribution<size_t> distribution;
    std::mt19937 engine;
public:
    RandElemGetter(container_type& container): containter_ref(container),distribution(0,container.size()-1)
    {

    }
    element_type get()
    {
        return containter_ref[distribution(engine)];
    }
};

usage :

{
vector<int> v{1,2,3,1701,1729};
vector<int> result;
RandElemGetter<vector<int>,int> reg_v(v);
for(size_t i=0;i<nelements;++i)
result.push_back(reg_v.get());
}

So is this OK solution? I know it is not thread safe, that is not the point. Im wondering is there better "scoped" way of getting random element from random access container. It could be modified maybe with std::advance to work for all.

Answer 1

RandElemGetter(container_type container):
   containter_ref(container),distribution(0,container.size()-1)

This takes the container by value, creating a temporary copy, and stores a reference to that copy. The reference is invalid once the constructor has finished.

You need to either store a copy, or pass the argument by reference. It's a good idea to pass complex objects by constant reference anyway, to avoid unnecessary copying.

Answer 2

• I would use <typename container_type, typename value_type = container_type::typename value_type>. STL containers have typedefs for their value_types, so there's usually no need to repeat yourself.
• The correct solution is indeed to return *std::advance(containter_ref.begin(), distribution(engine));
• I'd rename get() to operator(), and provide a result_type typedef to comply with the STL model AdaptableGenerator

You might want to have a peak at SGI's random_sample from the original STL; I believe GNU still has that as an extension.