priority_queue in c++ with custom comparator

Question

I created a priority_queue and define a custom comparator(a struct) which takes an 2D vector in the constructor. It compiles and runs successfully to the point of declaring that priority queue. However, if I try to add an element into the priority queue, it pops me error:

error: request for member 'push' in 'minHeap', which is of non-class type 'std::priority_queue, std::vector >, MyComparator>(MyComparator)' minHeap.push(make_pair(0, 1));

I would like to know what is the problem?

#include <iostream>
#include <vector>
#include <queue>
using namespace std;



struct MyComparator{
    vector<vector<int>> array2D;

    MyComparator(const vector<vector<int>>& arrays){
        array2D = arrays;   
    }

    bool operator()(const pair<int, int> &p1, const pair<int, int> &p2){
        return array2D[p1.second][p1.first] > array2D[p2.second][p2.first];
    }
};


int main() {
    vector<vector<int>> arrays = {{1, 3, 5, 7}, {2, 4, 6},{0, 8, 9, 10, 11}};
    priority_queue<pair<int, int>, vector<pair<int, int>>, MyComparator> minHeap(MyComparator(arrays));

//work well without error above

    minHeap.push(make_pair(0, 1)); //pop me error
    return 0;
}

Possible duplicate of [Most vexing parse: why doesn't A a(()); work?](https://stackoverflow.com/questions/1424510/most-vexing-parse-why-doesnt-a-a-work) — 1201ProgramAlarm, May 26 '17 at 02:42

score 3 · Answer 1 · answered May 26 '17 at 02:32

This is because your priority queue initialization is being treated as a function declaration and not a variable declaration. See https://en.wikipedia.org/wiki/Most_vexing_parse for reasons as to why.

Change the line where you initialize your priority queue to this

priority_queue<pair<int, int>, vector<pair<int, int>>, MyComparator> minHeap{MyComparator(arrays)};

and it should work fine.

The reason it works now is because the {} is not ambiguous anymore. The {} was introduced in C++11 as a way to construct objects, it is in many cases preferable to parentheses to initialize a variable. For more see Why is list initialization (using curly braces) better than the alternatives?

Note For the future, always remember to compile with the warning flags enabled. The usual recommended ones are -Werror, -Wall and -pedantic. When you enable those flags, the compiler will alert you to the ambiguity (the most vexing parse) present in your code.

This should work. If not, try uniform initialization for both `minHeap` and the unnamed `MyComparator` and see: `priority_queue, vector>, MyComparator> minHeap{MyComparator{arrays}};` — nakiya, May 26 '17 at 03:11

score 2 · Accepted Answer · answered May 26 '17 at 02:41

This problem is known as the most vexing parse.

priority_queue<pair<int, int>, vector<pair<int, int>>, MyComparator> minHeap(MyComparator(arrays));

Here, the compiler cannot decide if this is a function signature or the invocation of a constructor. The usual way to get rid of this ambiguity is by adding a pair of extra parentheses:

priority_queue<pair<int, int>, vector<pair<int, int>>, MyComparator> minHeap((MyComparator(arrays)));

Curious's answer is also correct, but there are different takes on using braces for constructor invocation.

This works! I have never met this kind of problem, but now I know. Thank you! I am happy that I learnt something!! — bunny, May 26 '17 at 03:05

priority_queue in c++ with custom comparator

2 Answers2