c++ vector of strings - issue with pointers

I'm trying to write a method that gets a vector of strings and returns a pointer to a random element. Can you please tell what is the problem with the following code?

string* getRandomOption(vector<string> currOptions){
    vector<string>::iterator it;
开发者_JAVA技巧    it=currOptions.begin(); 
    string* res;

    int nOptions = currOptions.size();

    if(nOptions != 1){
        int idx = rand() % (nOptions-1);

        while (idx!=0){
            it++;
            idx--;
        };

    };


    res = &(*it);
};

Thanks, Li

Why return a pointer? Keep it simple!

std::string random_option(const std::vector<std::string>& options)
{
    return options[rand() % options.size()];
}

And since this works for any type, not only strings, I would prefer a generic solution:

template <typename T>
T random_element(const std::vector<T>& options)
{
    return options[rand() % options.size()];
}

If you want to "returns a pointer to a random element" you need to pass a reference to your vector. For now, it is copied !

You should do :

string* getRandomOption(vector<string> & currOptions)

By the way there is no return in your function for the moment, you need to add a return statement to send your pointer.

You are passing the vector by value, i.e. the function has a local copy of the original vector. Then you [intend to] return a pointer to an element in this vector. But when you return from the function, this local copy is destroyed, and your pointer is left dangling.

Better version of the same, because it works with any container, rather than vectors only. Here's the C++03 version:

template <typename ForwardIterator>
ForwardIterator random_element(ForwardIterator begin, ForwardIterator end)
{
    typename std::iterator_traits<ForwardIterator>::difference_type
        size = std::distance(begin, end);
    if (size) //divide by zero errors are bad
        std::advance(begin, std::rand() % size);
    return begin;
}

If you're on C++11, you can replace the above with this:

template <typename ForwardIterator>
ForwardIterator random_element(ForwardIterator begin, ForwardIterator end)
{
    auto size = std::distance(begin, end);
    if (size) //divide by zero errors are bad
        std::advance(begin, std::rand() % size);
    return begin;
}

Which gets around the std::iterator_traits<t>::difference_type glue.

maybe you want to change your function prototype to:

const string* getRandomOption(const vector<string>& currOptions)

or

string* getRandomOption(vector<string>& currOptions)

or else you just get an element from a temporarily copy.

Aside from the other problems mentioned in the other answers, when a vector resizes, any pointers or iterators (or references of any nature) become invalid. Don't return a pointer.