Trying to sort map by values but get errors

98 views Asked by At

I have this program with a map and i'm trying to sort them by values but i got errors. Can anyone tell me what I do wrong. Errors are at 28, 29, 30 line. Thanks

#include <iostream>
#include <cstdlib>
#include <map>
#include <string>
#include <vector>
#include <algorithm>
using namespace std;
typedef pair<string,int> pair;
void CovidMapa()
{
    typedef map <string,int> covid1304;
    covid1304 cd;
    cd["Kumanovo"] = 44;
    cd["Skopje"] = 28;
    cd["Prilep"] = 11;
    cd["Bitola"] = 6;
    cd["Tetovo"] = 5;
    cd["Veles"] = 4;
    cd["Debar"] = 2;
    cd["Gostivar"] = 2;
    cd["Stip"] = 1;
    cd["Kavadarci"] = 1;
    covid1304::iterator ir;
    for(ir = cd.begin(); ir != cd.end(); ++ir)
    {
        cout<<"Grad: "<<ir->first<<" Zaboleni: "<<ir->second<<endl;
    }
    vector <pair> vec;
    copy (cd.begin(); cd.end(); back_inserter<vector<pair> >(vec));
    sort (vec.begin(), vec.end(), [](const pair& l, pair& r)
          {
              if(l.second != r.second)
              return l.second<r.second;
              return l.first<r.first;
          });
    for (auto const &pair: vec)
    {
        cout<<"Grad: "<<pair.first<<" Zaboleni: "<<pair.second;
    }
}

int main()
{
    CovidMapa();
}

||=== Build: Debug in NaprednoCovid (compiler: GNU GCC Compiler) ===|

C:\Users\Kent\Desktop\NaprednoCovid\main.cpp||In function 'void CovidMapa()':|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|28|error: template argument 1 is invalid|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|28|error: template argument 2 is invalid|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|28|error: invalid type in declaration before ';' token|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|29|error: expected ')' before ';' token|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|29|error: template argument 1 is invalid|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|29|error: template argument 2 is invalid|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|29|error: no matching function for call to 'back_inserter(int&)'|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|29|note: candidate is:|
c:\program files (x86)\codeblocks1\mingw\lib\gcc\mingw32\4.8.1\include\c++\bits\stl_iterator.h|479|note: template<class _Container> std::back_insert_iterator<_Container> std::back_inserter(_Container&)|
c:\program files (x86)\codeblocks1\mingw\lib\gcc\mingw32\4.8.1\include\c++\bits\stl_iterator.h|479|note:   template argument deduction/substitution failed:|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|29|error: template argument 1 is invalid|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|29|error: expected ';' before ')' token|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|30|error: request for member 'begin' in 'vec', which is of non-class type 'int'|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|30|error: request for member 'end' in 'vec', which is of non-class type 'int'|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|30|error: reference to 'pair' is ambiguous|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|8|note: candidates are: typedef struct std::pair<std::basic_string<char>, int> pair|
c:\program files (x86)\codeblocks1\mingw\lib\gcc\mingw32\4.8.1\include\c++\bits\stl_pair.h|96|note:                 template<class _T1, class _T2> struct std::pair|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|30|error: 'pair' does not name a type|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|35|error: ISO C++ forbids declaration of 'parameter' with no type [-fpermissive]|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|35|error: expected '{' before ';' token|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp||In function 'void CovidMapa()':|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|35|warning: lambda expressions only available with -std=c++11 or -std=gnu++11 [enabled by default]|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|35|error: expected ')' before ';' token|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|36|warning: 'auto' changes meaning in C++11; please remove it [-Wc++0x-compat]|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|36|error: ISO C++ forbids declaration of 'pair' with no type [-fpermissive]|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|36|error: range-based 'for' loops are not allowed in C++98 mode|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|38|error: request for member 'first' in 'pair', which is of non-class type 'const int'|
C:\Users\Kent\Desktop\NaprednoCovid\main.cpp|38|error: request for member 'second' in 'pair', which is of non-class type 'const int'|
||=== Build failed: 20 error(s), 2 warning(s) (0 minute(s), 0 second(s)) ===|
2

There are 2 answers

6
Vlad from Moscow On BEST ANSWER

For starters there are typos

copy (cd.begin(); cd.end(); back_inserter<vector<pair> >(vec));
               ^^^       ^^^                    

Secondly for unqualified name lookup due to the using directive you have two entities with the same name pair. So the compiler issues an error.

Write for example

typedef pair<string,int> Pair;

//...

vector <Pair> vec;
copy (cd.begin(), cd.end(), back_inserter(vec));
sort (vec.begin(), vec.end(), [](const Pair& l, const Pair& r)
      {
          if(l.second != r.second)
          return l.second<r.second;
          return l.first<r.first;
      });

Another approach is to leave the typedef declaration as is but then to use the qualified name ::pair instead of the unqualified name pair as for example

vector <::pair> vec;

Here is your program with the shown updates.

#include <iostream>
#include <cstdlib>
#include <map>
#include <string>
#include <vector>
#include <algorithm>

using namespace std;
typedef pair<string,int> pair;

void CovidMapa()
{
    typedef map <string,int> covid1304;
    covid1304 cd;
    cd["Kumanovo"] = 44;
    cd["Skopje"] = 28;
    cd["Prilep"] = 11;
    cd["Bitola"] = 6;
    cd["Tetovo"] = 5;
    cd["Veles"] = 4;
    cd["Debar"] = 2;
    cd["Gostivar"] = 2;
    cd["Stip"] = 1;
    cd["Kavadarci"] = 1;
    covid1304::iterator ir;
    for(ir = cd.begin(); ir != cd.end(); ++ir)
    {
        cout<<"Grad: "<<ir->first<<" Zaboleni: "<<ir->second<<endl;
    }
    vector <::pair> vec;
    copy (cd.begin(), cd.end(), back_inserter(vec));
    sort (vec.begin(), vec.end(), [](const ::pair& l, const ::pair& r)
          {
              if(l.second != r.second)
              return l.second<r.second;
              return l.first<r.first;
          });

    cout << '\n';          
    for (auto const &pair: vec)
    {
        cout<<"Grad: "<<pair.first<<" Zaboleni: "<<pair.second << endl;
    }
}

int main()
{
    CovidMapa();
}
0
Ted Lyngmo On

You typedef pair but pair is already in the global namespace because of using namespace std; Don't do using namespace std;.

You also have ; instead of , in std::copy, but there's no need to use std::copy at all. Just create the vector from the iterators directly:

vector <mypair> vec(cd.begin(), cd.end());

I also recommend using std::tie in your comparison lambda:

return std::tie(l.second, l.first) < std::tie(r.second, r.first);

Full code:

#include <algorithm>
#include <cstdlib>
#include <iostream>
#include <map>
#include <string>
#include <vector>
#include <tuple>

using namespace std; // don't do this
typedef pair<string,int> mypair; // rename your pair to not collide with std::pair
void CovidMapa()
{
    typedef map <string,int> covid1304;
    covid1304 cd;
    cd["Kumanovo"] = 44;
    cd["Skopje"] = 28;
    cd["Prilep"] = 11;
    cd["Bitola"] = 6;
    cd["Tetovo"] = 5;
    cd["Veles"] = 4;
    cd["Debar"] = 2;
    cd["Gostivar"] = 2;
    cd["Stip"] = 1;
    cd["Kavadarci"] = 1;
    covid1304::iterator ir;
    for(ir = cd.begin(); ir != cd.end(); ++ir)
    {
        cout<<"Grad: "<<ir->first<<" Zaboleni: "<<ir->second<<endl;
    }

    // construct vec directly from the iterators:
    vector <mypair> vec(cd.begin(), cd.end());

    sort (vec.begin(), vec.end(), [](const mypair& l, mypair& r)
          {
              // use std::tie to make the lambda easier to understand and maintain
              return std::tie(l.second, l.first) < std::tie(r.second, r.first);
          });

    for (auto const &pair: vec)
    {
        cout<<"Grad: "<<pair.first<<" Zaboleni: "<<pair.second << '\n';
    }
}

int main()
{
    CovidMapa();
}

Another option that doesn't require copying all the std::strings and ints can be to store iterators in your vector. Since iterators are light-weight, this approach is likely to be a little faster.

    vector <covid1304::iterator> vec;
    vec.reserve(cd.size());
    for(auto it=cd.begin(); it != cd.end(); ++it) vec.push_back(it);

    sort (vec.begin(), vec.end(), [](const auto& l, const auto& r)
          {
              // use std::tie to make the lambda easier to understand and maintain
              return std::tie(l->second, l->first) < std::tie(r->second, r->first);
          });

    for (auto const &pair: vec)
    {
        cout<<"Grad: "<<pair->first<<" Zaboleni: "<<pair->second << '\n';
    }