Sort unique values from a map>

Question

I have a custom map in my header file

class Tramway
{
private:
    using stations = std::vector <std::string>;
    using Tramlines = std::map <std::string, stations>;
...../

I'm trying to sort the unique values, but so far my approach is giving me compile errors. Here is my code.

void Tramway::print_stations(const Tramway::Tramlines &tramlines)
{

    for(auto map_iter = tramlines.cbegin(); map_iter != tramlines.cend(); ++map_iter)
    {
        std::unique(map_iter->second.cbegin(), map_iter->second.cend());

        std::sort(map_iter->second.begin(), map_iter->second.end());

       for( auto vec_iter = map_iter->second.cbegin() ; vec_iter != map_iter->second.cend() ; ++vec_iter )
           std::cout << *vec_iter << std::endl;
    }

}

Is there a way to get unique values and sort them at the same time? I tried std::sort(std::unique(..,..)) but std::sort() needs two arguments to work.

Below are the errors I keep getting

    x86_64-w64-mingw32\7.3.0\include\c++\bits\stl_algobase.h:148: error: no matching function for call to 'swap(const std::__cxx11::basic_string<char>&, const std::__cxx11::basic_string<char>&)'
           swap(*__a, *__b);
           ~~~~^~~~~~~~~~~~

x86_64-w64-mingw32\7.3.0\include\c++\bits\stl_algo.h:975: error: passing 'const std::__cxx11::basic_string<char>' as 'this' argument discards qualifiers [-fpermissive]
    *++__dest = _GLIBCXX_MOVE(*__first);
              ^

x86_64-w64-mingw32\7.3.0\include\c++\bits\stl_algo.h:1852: error: passing 'const std::__cxx11::basic_string<char>' as 'this' argument discards qualifiers [-fpermissive]
        *__first = _GLIBCXX_MOVE(__val);
                 ^

x86_64-w64-mingw32\7.3.0\include\c++\bits\stl_heap.h:252: error: passing 'const std::__cxx11::basic_string<char>' as 'this' argument discards qualifiers [-fpermissive]
       *__result = _GLIBCXX_MOVE(*__first);
                 ^

x86_64-w64-mingw32\7.3.0\include\c++\bits\move.h:187: error: no type named 'type' in 'struct std::enable_if<false, void>'

86_64-w64-mingw32\7.3.0\include\c++\bits\stl_heap.h:225: error: passing 'const std::__cxx11::basic_string<char>' as 'this' argument discards qualifiers [-fpermissive]
    *(__first + __holeIndex) = _GLIBCXX_MOVE(*(__first + __secondChild));
                             ^

86_64-w64-mingw32\7.3.0\include\c++\bits\stl_heap.h:231: error: passing 'const std::__cxx11::basic_string<char>' as 'this' argument discards qualifiers [-fpermissive]
    *(__first + __holeIndex) = _GLIBCXX_MOVE(*(__first
                             ^
x86_64-w64-mingw32\7.3.0\include\c++\bits\stl_algobase.h:548: error: passing 'const std::__cxx11::basic_string<char>' as 'this' argument discards qualifiers [-fpermissive]
      *--__result = std::move(*--__last);
      ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~

You're supposed to call erase after calling std::unique ([see this page](https://en.cppreference.com/w/cpp/algorithm/unique)). Also `but so far my approach is giving me compile errors` you forgot to include the compiler error. — Borgleader, Oct 31 '19 at 12:16
std::unique removes only consecutive duplicates, leaving tail of container in undetermined state — Swift - Friday Pie, Oct 31 '19 at 12:25
`std::set` is a sorted, unique collection. Would a change in data structure from ```std::vector``` to this avoid the need to rearrange in the first place? — parktomatomi, Oct 31 '19 at 13:03

L. F. · Answer 1 · 2019-11-01T08:24:26.547

Three problems:

std::unique requires modifiable iterators. You should not pass cbegin/cend to them. The function should also take a modifiable reference.
std::sort first, std::unique after. std::unique only removes adjacent equivalent elements.
std::unique does not have permission to change the size of the vector. It merely places the resulted sequence at the beginning of the vector. Use the remove-erase idiom to eliminate the removed elements.

Fixed code:

void Tramway::print_stations(Tramway::Tramlines& tramlines)
{
    for (auto map_iter = tramlines.cbegin(); map_iter != tramlines.cend(); ++map_iter)
    {
        std::sort(map_iter->second.begin(), map_iter->second.end());
        map_iter->second.erase(std::unique(map_iter->second.begin(), map_iter->second.end()), map_iter->second.end());

       for (auto vec_iter = map_iter->second.cbegin(); vec_iter != map_iter->second.cend(); ++vec_iter)
           std::cout << *vec_iter << '\n';
    }    
}

Also, consider using range-based for loops and a reference to simplify the code:

void Tramway::print_stations(Tramway::Tramlines& tramlines)
{
    for (auto& tramline : tramlines) {
        auto& station = tramline.second;
        std::sort(station.begin(), station.end());
        station.erase(std::unique(station.begin(), station.end()), station.end());

        std::copy(station.begin(), station.end(),
                  std::ostream_iterator<stations>{std::cout, '\n'});
    }    
}

Ah I see, now it makes sense. I tried your code, but ´map_iter->second.erase´ gives an error, ´no matching ´member function for call to erase — cyberbemon, Oct 31 '19 at 13:28
@cyberbemon What else does it say? The error message should be longer than that. — L. F., Oct 31 '19 at 13:59
here is the full error `tramway.cpp:78:26: error: no matching member function for call to 'erase' stl_vector.h:1206:7: note: candidate function not viable: 'this' argument has type 'const std::vector, std::allocator > >', but method is not marked const stl_vector.h:1179:7: note: candidate function not viable: requires single argument '__position', but 2 arguments were provided` — cyberbemon, Oct 31 '19 at 14:03
@cyberbemon The function argument (tramline) has to be const also. — L. F., Nov 01 '19 at 08:24

Sort unique values from a map>

1 Answers1