2

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;
}
General Grievance
  • 4,555
  • 31
  • 31
  • 45
sekogs
  • 292
  • 4
  • 18
  • in the beginning of my_alg, why are you doing "Edge *find_edge();" ? – ClickerMonkey Jan 06 '12 at 18:34
  • If it is any help, I get this error: error C2660: 'find_edge' : function does not take 3 parameters, yet it is defined as : Edge *find_edge(Graph *g, Node *from, Node *to) , three params – Software_Designer Jan 06 '12 at 18:43
  • Actually i forget to delete *find_edge() in my_alg.i added this line when i try to solve the problem.but it does not change anything – sekogs Jan 06 '12 at 18:49
  • I do not get any error such as 'find_edge' : function does not take 3 parameters. – sekogs Jan 06 '12 at 20:16
  • 2
    To get good answers, it would be good if you could reduce your code to the parts that are explicitly necessary for your question. Not a lot of people will read through all your 300 lines of code. Furthermore, pleas do look into the formatting of the code - this time I did some indentation for you, which will be visible once it is peer reviewed. However, usually, this should be your job. – Thilo Jan 06 '12 at 21:16
  • Thanks a lot Thilo.I'm new at topic.So again thanks for your advise and help – sekogs Jan 06 '12 at 21:25
  • To narrow the problem a bit down: Does `cout << bok << endl;` return the same value before and after the second call to `find_edge`? I do not see the exact reason right now, but I assume your edge is implicitly copied at one place. – Thilo Jan 06 '12 at 21:31
  • when cout< – sekogs Jan 06 '12 at 21:36
  • sorry they give the same result when i cout< – sekogs Jan 06 '12 at 21:38
  • since both give same solution,as far as understand they point to same Edge.So it should give the same flow.But they give different number when i print the flow – sekogs Jan 06 '12 at 21:49
  • I still can not find the problem – sekogs Jan 07 '12 at 19:47
  • You are certain that both pointers are the same? Maybe, to be sure, just save the second pointer to another variable, say `bok2`, and then compare `bok == bok2`. Furthermore, you should still reduce your code - for example, the bfs-algorithm hardly has to be there. What member functions and variables are needed for the problem part to run? Remove everything else. – Thilo Jan 07 '12 at 20:08
  • Yes i try What you said.And they both are the same.Thanks again for your help. – sekogs Jan 07 '12 at 20:29
  • Although the both pointer have same value,at the second cout flow is always 0. – sekogs Jan 07 '12 at 20:39

1 Answers1

4

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.

Thilo
  • 8,827
  • 2
  • 35
  • 56