Shuffle a list of strings in C++

Question

I tried to shuffle the following list of strings:

list<string> l({"10000007", "1", "4", "5", "7", "12", "23", "25", "26", "27", "30", "31", "32", "44", "46", "47", "59", "65", "91"})

all my attempts have failed. This is one of my best attempts.

Try 1

Basically I copy from this answerd Randomize a std::list<std::string>

#include <iostream>
#include <functional>
#include <iterator>
#include <algorithm>
#include <string>
#include <list>
#include <vector>
#include <random>
#include <numeric>

int main() {
    std::list<std::string> l({"10000007", "1", "4", "5", "7", "12", "23", "25", "26", "27", "30", "31", "32", "44", "46", "47", "59", "65", "91"});
    std::vector<std::reference_wrapper<std::string>> v(l.cbegin(), l.cend());
    std::random_device rd;
    std::mt19937 generator(rd());
    std::shuffle(v.begin(), v.end(), generator);
    std::cout << "Original list:\n";
    std::copy(l.cbegin(), l.cend(), std::ostream_iterator<std::string>(std::cout, " "));
    std::cout << "\nShuffled view:\n";
    std::copy(v.cbegin(), v.cend(), std::ostream_iterator<std::string>(std::cout, " "));
}

I had this error trace:

In file included from /usr/include/c++/7/vector:62:0,
                 from /usr/include/c++/7/functional:61,
                 from prueba.cpp:2:
/usr/include/c++/7/bits/stl_construct.h: In instantiation of ‘void std::_Construct(_T1*, _Args&& ...) [with _T1 = std::reference_wrapper<std::__cxx11::basic_string<char> >; _Args = {const std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&}]’:
/usr/include/c++/7/bits/stl_uninitialized.h:83:18:   required from ‘static _ForwardIterator std::__uninitialized_copy<_TrivialValueTypes>::__uninit_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >; _ForwardIterator = std::reference_wrapper<std::__cxx11::basic_string<char> >*; bool _TrivialValueTypes = false]’
/usr/include/c++/7/bits/stl_uninitialized.h:134:15:   required from ‘_ForwardIterator std::uninitialized_copy(_InputIterator, _InputIterator, _ForwardIterator) [with _InputIterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >; _ForwardIterator = std::reference_wrapper<std::__cxx11::basic_string<char> >*]’
/usr/include/c++/7/bits/stl_uninitialized.h:289:37:   required from ‘_ForwardIterator std::__uninitialized_copy_a(_InputIterator, _InputIterator, _ForwardIterator, std::allocator<_Tp>&) [with _InputIterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >; _ForwardIterator = std::reference_wrapper<std::__cxx11::basic_string<char> >*; _Tp = std::reference_wrapper<std::__cxx11::basic_string<char> >]’
/usr/include/c++/7/bits/stl_vector.h:1331:33:   required from ‘void std::vector<_Tp, _Alloc>::_M_range_initialize(_ForwardIterator, _ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >; _Tp = std::reference_wrapper<std::__cxx11::basic_string<char> >; _Alloc = std::allocator<std::reference_wrapper<std::__cxx11::basic_string<char> > >]’
/usr/include/c++/7/bits/stl_vector.h:1299:23:   required from ‘void std::vector<_Tp, _Alloc>::_M_initialize_dispatch(_InputIterator, _InputIterator, std::__false_type) [with _InputIterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >; _Tp = std::reference_wrapper<std::__cxx11::basic_string<char> >; _Alloc = std::allocator<std::reference_wrapper<std::__cxx11::basic_string<char> > >]’
/usr/include/c++/7/bits/stl_vector.h:414:26:   required from ‘std::vector<_Tp, _Alloc>::vector(_InputIterator, _InputIterator, const allocator_type&) [with _InputIterator = std::_List_const_iterator<std::__cxx11::basic_string<char> >; <template-parameter-2-2> = void; _Tp = std::reference_wrapper<std::__cxx11::basic_string<char> >; _Alloc = std::allocator<std::reference_wrapper<std::__cxx11::basic_string<char> > >; std::vector<_Tp, _Alloc>::allocator_type = std::allocator<std::reference_wrapper<std::__cxx11::basic_string<char> > >]’
prueba.cpp:13:76:   required from here
/usr/include/c++/7/bits/stl_construct.h:75:7: error: binding reference of type ‘std::__cxx11::basic_string<char>&’ to ‘const std::__cxx11::basic_string<char>’ discards qualifiers
     { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }
       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In file included from /usr/include/c++/7/bits/std_function.h:44:0,
                 from /usr/include/c++/7/functional:58,
                 from prueba.cpp:2:
/usr/include/c++/7/bits/refwrap.h:334:7: note:   initializing argument 1 of ‘std::reference_wrapper<_Tp>::reference_wrapper(_Tp&) [with _Tp = std::__cxx11::basic_string<char>]’
       reference_wrapper(_Tp& __indata) noexcept
       ^~~~~~~~~~~~~~~~~

I tried to understand what reference_wrapper do in this example, without success.

Other tries

All my attempts with classical combinations have fail also... With random_engine, shuffle and srand with trace error of thousands of lines.

Why not convert to ints?

In my original code, there are two .json files, the jsons always have the keys in string format. I know that I can transform the list of strings into list of integers but the problem is that I'm creating and algorithm and there are:

• 2000 lists
• 2000 iterations in the algorithm
• You have to do several times this conversion per iteration (strings to int, int to string...)

So I think this option will be computationally very expensive.

Thanks in advance.

Answer 1

I'd start by questioning the use/need for using an std::list at all. In this case, you only seem to be using the list to initialize a vector. That being the case, you might as well just initialize and use the vector directly:

#include <iostream>
#include <functional>
#include <iterator>
#include <algorithm>
#include <string>
#include <list>
#include <vector>
#include <random>
#include <numeric>

int main() {
    std::vector<std::string> v({"10000007", "1", "4", "5", "7", "12", "23", "25", "26", "27", "30", "31", "32", "44", "46", "47", "59", "65", "91"});
    std::random_device rd;

    std::cout << "Original list:\n";
    std::copy(v.cbegin(), v.cend(), std::ostream_iterator<std::string>(std::cout, " "));

    std::mt19937 generator(rd());
    std::shuffle(v.begin(), v.end(), generator);

    std::cout << "\nShuffled view:\n";
    std::copy(v.cbegin(), v.cend(), std::ostream_iterator<std::string>(std::cout, " "));

    std::cout << "\n";    
}

If you really insist on using the list anyway, you can get rid of the reference wrapper and it'll work:

#include <iostream>
#include <functional>
#include <iterator>
#include <algorithm>
#include <string>
#include <list>
#include <vector>
#include <random>
#include <numeric>

int main() {
    std::list<std::string> l({"10000007", "1", "4", "5", "7", "12", "23", "25", "26", "27", "30", "31", "32", "44", "46", "47", "59", "65", "91"});
    std::vector<std::string> v(l.cbegin(), l.cend());
    std::random_device rd;
    std::mt19937 generator(rd());
    std::shuffle(v.begin(), v.end(), generator);
    std::cout << "Original list:\n";
    std::copy(l.cbegin(), l.cend(), std::ostream_iterator<std::string>(std::cout, " "));
    std::cout << "\nShuffled view:\n";
    std::copy(v.cbegin(), v.cend(), std::ostream_iterator<std::string>(std::cout, " "));
}

...but I don't see a lot of point to this.

As far as the expense of storing ints vs. strings: as long as you're using a relatively recent compiler so its (library's) implementation of std::string has the short string optimization, there's probably not a huge difference between the two. If you (might) need to support an older compiler that lacks short string optimization, that drastically increases the chances that you'll be better off converting and storing ints instead of storing strings.

The problem is pretty simple: without the short string optimization, each string will result in a heap allocation to store the actual data. That heap allocation may easily be slower than converting to and from int (but you'd need to test to be sure).

For those who really care a lot about speed, the ideal would probably be to memory map the entire json file, then create string_view objects for the values you care about. Then you can shuffle the string_view objects, and print them out (or whatever) as you see fit--but all of those just contain pointers into the original data, so you're not copying the underlying data all.

Answer 2

The cbegin() member function returns a const_iterator which when dereferenced returns a const std::string&.

Change the creation of your vector to:

std::vector<std::reference_wrapper<const std::string>> v(l.cbegin(), l.cend());

Demo

Answer 3

On this line:

std::vector<std::reference_wrapper<std::string>> v(l.cbegin(), l.cend());

you are using const_iterators to construct the vector, which causes a type mismatch error when constructing the reference_wrapper<std::string>s from a const std::string.

Instead, you need non-const iterators, like this:

std::vector<std::reference_wrapper<std::string>> v(l.begin(), l.end());

Here's a demo.