Question

I'm trying to implement the Ford Fulkerson Algorithm in C++.

However, I'm having trouble with my find_edge function. When I call this function in my_alg, it chooses the correct edge and then the flow is incremented in my_alg. It chooses the right edge and increment its flow (flow), but when I call the find_edge function again, the flow is not incremented as it should be.

This results in an endless loop of my algorithm. Probably I do something wrong with the pointers. You can see my code below.

//An object of this class represents an edge in the graph.
class Edge
{
private:
    //Node *prev;

public:
    int flow;

    Edge(Node *firstNode, Node *secNode, unsigned inCost) {
        orgNode = firstNode;
        dstNode = secNode;
        bridge_capacity = inCost;
    }

    Edge() {
        flow=0;
    }
};

//An object of this class holds a vertex of the graph
class Node
{
public:
    Node *prev;

    vector<Edge>& getAdjNodeList() {
        return adjNodeList;
    }
};

Edge *find_edge(Graph *g,Node *from,Node *to) {
    vector<Edge> b=from->getAdjNodeList();
    for(int i=0;i<b.size();i++) {
        if(b[i].getDstNode()==to)
            return (&b[i]);
    }
    return NULL;
}

int my_alg(Graph *as,Node *source,Node *sink){
    Edge *find_edge();

    int max_flow=0;

    while(bfs(as,source,sink)) {
        Node *b=as->nodeList[num_isl];
        int inc=100000000;
        while(b->prev!=NULL) {
            Edge *bok=find_edge(as,b->prev,b);
            inc=min(inc,bok->get_bridge_capacity()-bok->flow);
            b=b->prev;
        }

        b=as->nodeList[num_isl];

        while(b->prev!=NULL){
            Edge *bok = find_edge(as,b->prev,b);
            bok->flow += inc;       // This is the place the flow is incremented
            bout << bok->flow;      // Here, everything is alright.
            bok = find_edge(as,b->prev,b);
            cout << bok->flow;      // However, this is is not the correct result.
        }
        max_flow+=inc;
    }
    return max_flow;
}
Was it helpful?

Solution

I had a more thorough look at your code. To help you track your problems down yourself in the future, I will show you a sample process of finding the error.

If you really can not find the problem by looking at the code, you may want to strip down everything that obfuscates your view on the problem. The reduced code could look like this:

class Edge {
public:
    int flow;
};    

class Node {
private:
    vector<Edge> adjNodeList; // list of outgoing edges for this vertex

public:
    vector<Edge> & getAdjNodeList() {
        return adjNodeList;
    }

    void addAdjNode(Node* newAdj) {
        adjNodeList.push_back(Edge(newAdj));
    }
 };    


int main() {
    Node *node1 = new Node();
    Node *node2 = new Node();
    node1->addAdjNode(node2);

    vector<Edge> t = node1->getAdjNodeList();
    vector<Edge> f = node1->getAdjNodeList();
    t[0].flow = 11;

    cout << t[0] << endl;
    cout << f[0] << endl;
}

If you would run this code, you would notice that t[0] and f[0] are not the same. As I just copied the crucial elements of your code, the reason should still be the same.

What is happening here? When calling

vector<Edge> t = node1->getAdjNodeList();

the adjacency list is returned by reference, which should leave you with a reference to the original list - you should be able to change it's elements, shouldn't you? However, when assigning this reference to the newly allocated vector t, the implicit copy constructor is called, thus t will contain a copy (!) of your vector while you wanted to save a reference.

To get around this problem, you could just have done the following:

vector<Edge> & t = node1->getAdjNodeList();

which saves the reference and does not create a new object.

I can only assume why the pointers happened to be identical between calls to the function: The object probably was copied to the same place every time. Furthermore, note that you increased the value of an object that did not exist anymore - the copy was deleted with the end of the find_edge-call.

It took some time to give an answer to your question as you did not track the problem down yourself. If you had given the example above, I bet your solution would have been there within a matter of minutes. You are encouraged to raise your problems here at stack overflow - however, most members will not be willing to work through a lot of code to identify the problem themselves. That means, high quality answers usually require questions that directly come to the point. (The last paragraph was intended to help you in the future, however, it could be reduced without altering the question).

Apart from that, I would strongly encourage you not to use your objects the way you do. By passing everything as references and making all changes outside the object, you essentially bypass the encapsulation that makes object orientated programming that powerful. For example, it would be much wiser (and would not have given you your problem) if you just had added another function increaseFlow(Edge* to, int increment) to your Node and had done everything within the object.

Hope I could help.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top