3

So basically I want to implement this Finding all cycles in undirected graphs in C++ (the only difference is that the graph is weighted), but that's not really my problem right now as I can probably deal with it later.

I tried to rewrite the C# code to C++, but I'm still not confident with my OOP in C++ and I don't quite understand what I did wrong. I used debugger and my program doesn't even enter the findNewCycles function and I'm also pretty sure that there are more problems, but currently I want to find out how to even start. There's something wrong with constructor of my Path class (at least debugger suggests me this), but I don't understand why. Can you please help me? Here's my code:

#include <iostream>
#include <utility>
#include <vector>
#include <algorithm>

using namespace std;

class Graph {
        struct Edge {
                int vert[2];
                double value;
            public:
                Edge(int vec1, int vec2, double w) {
                    vert[0] = vec1;
                    vert[1] = vec2;
                    value = w;
                };
        };

        struct Path {
                vector<int> vertices;
                double totalValue;
            public:
                Path() : totalValue(0) {};
                Path(vector<int> v, double tv) : vertices(v), totalValue(tv) {};
                Path(const Path &a) {
                    totalValue = a.totalValue;
                    vertices = a.vertices;
                }
        };

        int vortexCount, edgeCount, cycleCount;
        vector<Path> cycles;
        vector<Edge> edges;

        void findNewCycles(Path a) {
            int n = a.vertices[0];
            int x;
            Path sub(a);

            for(int i = 0; i < edgeCount; i++) {
                for(int j = 0; j <=1; j++) {
                    if (edges[i].vert[j] == n) {
                        x = edges[i].vert[(j+1)%2];

                        if (!visited(x, a)) {
                            sub.totalValue += edges[i].value;
                            sub.vertices.insert(sub.vertices.begin(), x);
                            findNewCycles(sub);
                        }
                        else if ((a.vertices.size() > 2) && (x == a.vertices[a.vertices.size() - 1])) {
                            Path normal = normalize(a);
                            Path inv = invert(normal);
                            if(isNew(normal) && isNew(inv)) cycles.push_back(normal);
                        }
                    }
                }
            }
        }

        bool equals(Path a, Path b) {
            if((a.vertices.size() == b.vertices.size()) && (a.totalValue == b.totalValue)) {
                for (unsigned i=0; i < a.vertices.size(); i++) {
                    if(a.vertices[i] != b.vertices[i]) return false;
                }
                return true;
            }
            else return false;
        }

        Path invert(Path a) {
            Path inverted(a);
            reverse(inverted.vertices.begin(), inverted.vertices.end());
            return normalize(inverted);
        }

        Path normalize(Path a) {
            Path normalized(a);
            vector<int>::iterator smallest = min_element(normalized.vertices.begin(), normalized.vertices.end());
            std::rotate(normalized.vertices.begin(), smallest, normalized.vertices.end());
            return normalized;
        }

        bool isNew(Path a) {
            for(int i=0; i<cycleCount; i++) {
                if(equals(cycles[i], a)) {
                    return false;
                }
            }
            return true;
        }

        bool visited(int n, Path a) {
            for (unsigned i=0; i < a.vertices.size(); i++) {
                if(a.vertices[i] == n) return true;
            }
            return false;
        }

    public:
        Graph(int size) : vortexCount(size), edgeCount(0), cycleCount(0) {};
        ~Graph() {};

        vector<Edge>::iterator findEdge(int v1, int v2) {
            if(v1 == v2 || v1 > vortexCount || v2 > vortexCount) return edges.end();
            vector<Edge>::iterator iter;

            for(iter = edges.begin(); iter != edges.end(); ++iter) {
                if(iter->vert[0] == v1 && iter->vert[1] == v2) return iter;
                if(iter->vert[1] == v1 && iter->vert[0] == v2) return iter;
            }
            return edges.end();
        }

        bool addEdge(int v1, int v2, double value) {
            if(v1 == v2 || v1 > vortexCount || v2 > vortexCount) return false;

            vector<Edge>::iterator p = findEdge(v1, v2);

            if(p != edges.end()) {
                p->value = value;
            }
            else {
                Edge edge(v1, v2, value);
                edges.push_back(edge);
                edgeCount++;
            }
            return true;
        }

        void runCycleSearch() {
            for (int i = 0; i < edgeCount; i++) {
                for (int j = 0; j < 2; j++) {
                    cout << i << " " << j;
                    Path searchPath;
                    searchPath.vertices.push_back(edges[i].vert[j]);
                    findNewCycles(searchPath);
                }
            }

            for(int i=0; i<cycleCount; i++) {
                for(unsigned j=0; j<cycles[i].vertices.size(); j++) {
                    cout << cycles[i].vertices[j] << " ";
                }
                cout << cycles[i].totalValue;
            }
        }

};

int main() {
    int n, v1, v2;
    double val;
    bool control = true;

    cin >> n;
    Graph graph(n);

    while(control) {
        cin >> v1;
        if(v1 == -1) break;
        cin >> v2 >> val;

        control = graph.addEdge(v1, v2, val);
    }

    graph.runCycleSearch();
}
Community
  • 1
  • 1
paolostyle
  • 1,849
  • 14
  • 17
  • So apparently I can't use the debugger (the fact that it's 5:30 AM here doesn't help) - it does enter the findNewCycles function, I'm not sure (yet) what's the problem with not displaying any cycles though - cycles.push_back(normal) has been executed for sure. – paolostyle Jan 22 '16 at 04:34

2 Answers2

0

Are you sure you are getting out of the while loop?

Try adding this line after you've read the input:

cout << "v1=" << v1 << ", v2=" << v2 << " val=" << val << endl;

and changing while (control) to

for (int i=0; i < n; i++)
Sandeep
  • 98
  • 8
  • n is a vertex count, so you can't change `while (control)` to `for (int i=0; i < n; i++)` because n has different meaning – valdem Jan 22 '16 at 13:45
0

I have found first mistake. You didn't update cycleCount variable when you find a cycle. I.e.

                    else if ((a.vertices.size() > 2) && (x == a.vertices[a.vertices.size() - 1])) {
                        Path normal = normalize(a);
                        Path inv = invert(normal);
                        if(isNew(normal) && isNew(inv)) cycles.push_back(normal);
                    }

replace with

                    else if ((a.vertices.size() > 2) && (x == a.vertices[a.vertices.size() - 1])) {
                        Path normal = normalize(a);
                        Path inv = invert(normal);
                        if (isNew(normal) && isNew(inv)) {
                            cycleCount++;
                            cycles.push_back(normal);
                        }

                    }

My advice only for this mistake: remove edgeCount and cycleCount at all, you can reach count of them by using cycles.size() and edges.size()

Also I changed printing to cout a little bit:

in void runCycleSearch() function:

    cout << "Cycles:\n";
    for (int i = 0; i<cycleCount; i++) {
        for (unsigned j = 0; j<cycles[i].vertices.size(); j++) {
            cout << cycles[i].vertices[j] << " ";
        }
        cout << cycles[i].totalValue;
        cout << "\n";
    }

But this code is very careless and full of mistakes. For example on case

4
1 2 0
2 3 0
3 4 0
1 4 0

It returns

Cycles:
1 2 4 0
1 3 2 0
2 4 3 0
1 4 3 0

I can continue to correct this code, but for me it is easier to rewrite from scratch :) There are a lot of problems:

  1. Graph represented as list of edges, so finding particular edge takes O(E) time. This is very inefficient, replace it with adjacency lists
  2. Try to avoid copying of objects, every vector is copying, for example isNew(Path a) you need replace it to references where it possible.
  3. Presence of vertex in path checked in linear time. Use global set or unordered set for this purpose. You also can use global path variable and don't need to copying it every recursive call.
valdem
  • 755
  • 5
  • 10