0

So I have been trying to implement the Dijkstra Algorithm for shortest path in a directed graph using adjacency lists, but for I don't know what reason, it doesn't print out the results (prints the minimum distance as 0 to all nodes).

The code I wrote is:

#include <fstream>
#include <functional>
#include <climits>
#include <vector>
#include <queue>
#include <list>

using namespace std;

struct node {
    int vertex;
    int weight;
    node(int v, int w) : vertex(v), weight(w) { };
    node() { }
};

class CompareGreater {
    public:
        bool const operator()(node &nodeX, node &nodeY) {
            return (nodeX.weight > nodeY.weight) ;
        }
};

vector< list<node> > adj;
vector<int> weights;
priority_queue<node, vector<node>, CompareGreater> Q;

int nrVertices, nrEdges;

void readData();
void Dijkstra(node);
void writeData();

int main(int argc, char *argv[]) {

    readData();
    Dijkstra(node(1, 0));
    writeData();

    return 0;
}

void readData() {
    fstream in("dijkstra.in", ios::in);

    int nodeX, nodeY, weight;

    in >> nrVertices >> nrEdges;

    adj.resize(nrVertices+1);
    weights.resize(nrVertices+1);

    for (int i = 1; i <= nrVertices; ++i) {
        weights.push_back(INT_MAX);
    }

    for (int i = 1; i <= nrEdges; ++i) {
        in >> nodeX >> nodeY >> weight;
        adj[nodeX].push_back(node(nodeY, weight));
    }

    in.close();
}

void Dijkstra(node startNode) {
    node currentNode;

    weights[startNode.vertex] = 0;
    Q.push(startNode);

    while (!Q.empty()) {
        currentNode = Q.top();
        Q.pop();

        if (currentNode.weight <= weights[currentNode.vertex]) {
            for (list<node>::iterator it = adj[currentNode.vertex].begin(); it != adj[currentNode.vertex].end(); ++it) {
                if (weights[it->vertex] > weights[currentNode.vertex] + it->weight) {
                    weights[it->vertex] = weights[currentNode.vertex] + it->weight;
                    Q.push(node((it->vertex), weights[it->vertex]));
                }
            }
        }
    }
}

void writeData() {
    fstream out("dijkstra.out", ios::out);

    weights.resize(nrVertices+1);
    for (vector<int>::iterator it = weights.begin()+1; it != weights.end(); ++it) {
        out << (*it) << " ";
    }

    out.close();
}

The input data was:

5 7
1 2 10
1 3 2
1 5 100
2 4 3
3 2 5
4 3 15
4 5 5

It means there are 5 nodes, 7 arcs (directed edges), and the arcs exist from node 1 to 2 with the cost of 10, from 1 to 3 with the cost of 2, and so on.

However, the output is wrong. I have no idea where the program might fail. I took the main idea from here: http://community.topcoder.com/tc?module=Static&d1=tutorials&d2=standardTemplateLibrary2#dijkstra1 (At the end it gives the idea for Dijkstra's Algorithm using a priority_queue).

Thanks in advance.

Raul

  • One of the better questions/answers i've seen covering the algorithm implemented in C++ [can be found here](http://stackoverflow.com/questions/3447566/dijkstras-algorithm-in-c/3448361#3448361). You may find several of the answers useful. – WhozCraig Feb 28 '14 at 15:51
  • The reason why I chose to post this question is because I do have the correct algorithm implemented. It is taken from TopCoder. So that's not the problem. The problem is that somewhere in my code, something happens which causes it to print false results. I assume it is because the use of the dynamic type list and more specifically vector >, together with priority_queue >. I mean, most probably I have a logical error somewhere, related to the use of these "complex" data structures (not complex, but much more complex than the simple Dijkstra approach in O(N^2) complexity) –  Feb 28 '14 at 15:54
  • Took a look at that link you attached. It is not a very good explanation, at least not as good as the one on TopCoder (where the whole idea is explained, and implemented in 3 different ways - standard, priority_queue >, set - but I still can't figure out why my program doesn't work properly (ie: print the correct result). If anyone can help me with it, I would be really grateful. Thanks in advance. –  Feb 28 '14 at 16:00
  • Interesting, because one of the answers *is* from TopCoder. – WhozCraig Feb 28 '14 at 16:01
  • Hm, well, I took a look on the code, it doesn't really seem to be anything like this: vi D(N, 987654321); priority_queue, greater > Q; D[0] = 0; Q.push(ii(0,0)); while(!Q.empty()) { ii top = Q.top(); Q.pop(); int v = top.second, d = top.first; if(d <= D[v]) { tr(G[v], it) { int v2 = it->first, cost = it->second; if(D[v2] > D[v] + cost) { D[v2] = D[v] + cost; //etc. –  Feb 28 '14 at 16:04
  • Go to this link, press CTRL+F "Dijstra via priority_queue" (that's the one I used) http://community.topcoder.com/tc?module=Static&d1=tutorials&d2=standardTemplateLibrary2#dijkstra1 –  Feb 28 '14 at 16:06
  • Also, unrelated, but make your comparator const. not its return value, the actual operator. Its parameters as well. – WhozCraig Feb 28 '14 at 16:07
  • Isn't it already const? EDIT: sorry, misread your comment. –  Feb 28 '14 at 16:08
  • No the comparator operator needs to be. Ideally likewise with the params (they're not modified). Every hint you can send to the optimizer to inline that thing the better. the operator should be `bool operator()(const node&, const node&) const {...}` – WhozCraig Feb 28 '14 at 16:10
  • Ah, I've never actually worked with this before, it's the first time I do, so I didn't know about it. Thanks :) –  Feb 28 '14 at 16:11
  • Thanks again, I really appreciate your fast and prompt answers :) –  Feb 28 '14 at 16:13

1 Answers1

2

The problem is in the line

weights.resize(nrVertices+1);

in readData(). This sets up a vector with nrVertices+1 elements of value 0. Later on, you append the actual values you want to this vector using weights.push_back(INT_MAX);.

In the actual Dijkstra algorithm, all interesting weights are thus 0, instead of the INT_MAX you want.

Replace the line with

weights.resize(1);

(just to make sure that the index 1 really refers to the first element - you seem to use 1 as the first index instead of 0), and it might work.

Frank
  • 1,565
  • 1
  • 10
  • 9
  • Yes indeed, thank you very much sir. I didn't pay enough attention to that part of the code. I thought the problem came from the use of the priority_queue or dynamic lists. Thanks again. I really appreciate it :) –  Feb 28 '14 at 16:03