Using reverse_iterator instead of const_reverse_iterator and getting nasty compiler warnings and errors

Question

I am currently in the process of learning C++ and encounter an issue while using

std::string::reverse_iterator 

to reverse a string. I get nasty compiler errors when trying to run the function below. However, when I switch to using,

std::string::const_reverse_iterator

,the code compiles and runs successfully. Why is this the case, especially when the documentation for the language says that reverse iterators can be declared and used? What if I need to say, remove elements from a string while looping through it in reverse, and want to use a reverse iterator? A

const_reverse_iterator

surely would not suffice in this case. Any help would be much appreciated. :)

std::string reverse(const std::string &str)
{
    std::string::reverse_iterator r_iter;
    std::string result;

    for (r_iter = str.rbegin(); r_iter < str.rend(); r_iter++) {
            result += (*r_iter);
    }

    return result;
}

Some of these errors are:

/usr/include/c++/7/bits/stl_iterator.h: In instantiation of ‘std::reverse_iterator<_Iterator>::reverse_iterator(const std::reverse_iterator<_Iter>&) [with _Iter = __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >; _Iterator = __gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >]’:
chap6.cpp:40:34:   required from here
/usr/include/c++/7/bits/stl_iterator.h:148:22: error: no matching function for call to ‘__gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >::__normal_iterator(std::reverse_iterator<__gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> > >::iterator_type)’
  : current(__x.base()) { }

and

/usr/include/c++/7/bits/stl_iterator.h:775:26: note:   candidate expects 0 arguments, 1 provided
/usr/include/c++/7/bits/stl_iterator.h:760:11: note: candidate: constexpr __gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >::__normal_iterator(const __gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >&)
     class __normal_iterator
           ^~~~~~~~~~~~~~~~~
/usr/include/c++/7/bits/stl_iterator.h:760:11: note:   no known conversion for argument 1 from ‘std::reverse_iterator<__gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> > >::iterator_type {aka __gnu_cxx::__normal_iterator<const char*, std::__cxx11::basic_string<char> >}’ to ‘const __gnu_cxx::__normal_iterator<char*, std::__cxx11::basic_string<char> >&’

Answer 1

You have a const std::string, which means that you can only do const things to it. There are two overloads of std::string::rbegin():

reverse_iterator rbegin();
const_reverse_iterator rbegin() const;

The first is unavailable to you, but the second is not.

std::string reverse(const std::string &str)
{
    std::string result;

    for (auto r_iter = str.rbegin(); r_iter != str.rend(); r_iter++) {
            result += *r_iter;
    }

    return result;
}

Note that you don't even need the loop, because you can construct a std::string from a pair of iterators, see overload (6)

std::string reverse(const std::string &str)
{
    return /* std::string */ { str.rbegin(), str.rend() };
}

Answer 2

str is passed as const& so you cannot erase elements from it and you also cannot get a non-const iterator to it, if you want to modify it you need to remove the const:

std::string reverse(std::string &str)
                //  ^---------------------- no const if you want to modify it !!!
{
    std::string::reverse_iterator r_iter;
    std::string result;

    for (r_iter = str.rbegin(); r_iter < str.rend(); r_iter++) {
            result += (*r_iter);
    }

    return result;
}

It is a matter of const-correctness. You cannot get a non const iterator to a constant string, because that would allow you to modify elements of the string which is acutally const.

Also note that there is std::reverse that you can use to reverse a string in place. If you still want to keep the original your method could look like this:

std::string reverse(std::string str)
                //  ^ pass by value because we need a copy anyhow 
{
    std::reverse(str.begin(),str.end());    
    return str;
}

However, instead of first copying and then reversing, this can be done in one step, as shown in Caleths answer.

Answer 3

The whole const correctness issue user463035818 pointed out aside. You are exhibiting some unidiomatic (to C++) code writing habits.

For one, don't define r_iter before you need it, rather limit it to the scope of the loop. Beyond that, this is a case where you really don't care about the exact type of the iterator. You just want the correct iterator type from the member function.

So just use auto for the type of the iterator.

std::string reverse(const std::string &str)
{
    std::string result;

    for (auto r_iter = str.rbegin(); r_iter < str.rend(); r_iter++) {
            result += (*r_iter);
    }

    return result;
}

Now your code is const correct by default. And if you do try to modify the input string by mistake, hopefully the error will be much clearer than when simply choosing the wrong iterator type inadvertently.

Answer 4

Note that the str of your function is a const-type argument. The rbegin() should return a const-type iterator. This is reasonable.

The declaration of rbegin() is as follows:

      reverse_iterator rbegin();
const_reverse_iterator rbegin() const;

So you can remove the const keyword from the function's parameter list to make it run or modify the str via reverse_iterator.