1

In

class vertex{

    // ...

};

I'm tryng to set this public attribute as private.

vector<edge> edges;

But here

void addedge(int from, int to, int length=-1) {
    vertices[from].edges.push_back(edge(to, length));
}

And here

edge &e = vertices[u].edges[i];

I can't understand how I can have access to edge with a method like in other cases.

Complete code:

#define MAX_VER 1000
#define INFINITE 9999
#include <vector>
#include <queue>
#include <iostream>
using namespace std;
class edge{
    private:
        int to;
        int length;
    public:
        edge(int to, int length) : to(to), length(length) {}
        int getTo(){return to;};
        void setTo(int t){to=t;};
        int getL(){return length;};
        void setL(int l){length=l;};
};
class vertex{
    private:
        //vector<edge> edges;
        int dis;
        int prev;
    public:
        vector<edge> edges;

        vector<edge> get_edges(){return edges;}

        int getDis(){return dis;};
        void setDis(int d){dis=d;};
        int getPrev(){return prev;};
        void setPrev(int p){prev=p;};
};
class graph{
    private:
        vertex vertices[MAX_VER];
    public:
        void reset() {
            for (int i=0; i < MAX_VER; i++) {
                vertices[i].get_edges().clear();
                vertices[i].setDis(INFINITE);
                vertices[i].setPrev(-1);
            }
        }

        void addedge(int from, int to, int length=-1) {
            vertices[from].edges.push_back(edge(to, length));
        }
        typedef pair<int, int> pp;
        void dijkstra(int source) {
            priority_queue<pp, vector<pp>, greater<pp> > q;
            vertices[source].setDis(0);
            q.push(make_pair(0, source));
            while (!q.empty()) {
                int u = q.top().second;
                int dis = q.top().first;
                q.pop();
                if (dis > vertices[u].getDis())
                    continue;
                for (int i = 0; i < vertices[u].get_edges().size(); i++) {
                    edge &e = vertices[u].edges[i];
                    if (dis + e.getL() < vertices[e.getTo()].getDis()) {
                        vertices[e.getTo()].setDis(dis + e.getL());
                        vertices[e.getTo()].setPrev(u);
                        q.push(make_pair(vertices[e.getTo()].getDis(), e.getTo()));
                    }
                }
            }
            cout << "Distance from vertex 2 to 4 is: " << vertices[4].getDis() << endl;
        }
};
int main() {
    graph g;
    g.reset();
    g.addedge(0, 1, 5);
    g.addedge(0, 2, 9);
    g.addedge(0, 3, 4);
    g.addedge(0, 4, 6);
    g.addedge(1, 2, 2);
    g.addedge(1, 3, 5);
    g.addedge(1, 4, 7);
    g.addedge(2, 3, 1);
    g.addedge(2, 4, 8);
    g.addedge(3, 4, 3);
    g.dijkstra(2);
    return 0;
}
Fab
  • 71
  • 2
  • 7
  • Your get_edges method returns edges by value, you want to return them by reference. However the result will be practically the same as when you leave the edges public. I guess what you really want to do is to write methods like addEdge and getEdge to your vertex class to work with edges instead of exposing them publicly. – hynner Apr 04 '15 at 21:39
  • Use your `get_edges` function if you want access to the `edges` variable from outside the class. – Andreas Vennström Apr 04 '15 at 21:41
  • `vector getEgdes() const` should do the job –  Apr 04 '15 at 21:41

1 Answers1

1

edges should be a private member, accessible via public get function. You have defined something like this (get_edges()), but it's not defined properly:

class vertex
{
    //...

public:
    vector<edge> edges;

    vector<edge> get_edges(){return edges;}

    //...
};

You return member via value, which means, that every time this function is called, a new copy of edges is created and that copy is returned!

You should return it by reference and provide version for both const and non-const object:

class vertex
{
    //...

public:
    vector<edge> edges;

    vector<edge>& get_edges() { return edges; }
    const vector<edge>& get_edges() const { return edges; }

    //...
};

Also, edges is used only by class graph. So probably it would be enough to declare friendship between these two classes?

Change vertex class this way:

class vertex
{
    friend class graph; //declare friend

private:
    int dis;
    int prev;
    vector<edge> edges; //make 'edges' private

public:
   vector<edge>& get_edges() { return edges; }
   const vector<edge>& get_edges() const { return edges; }
};

From now on, graph class will have access to all private and protected members of vertex class, because it has been declared as its friend. In simple cases, this is the fastest and less-intrusive solution (but only in terms of additional code, that needs to be written).

Mateusz Grzejek
  • 11,698
  • 3
  • 32
  • 49
  • Many thanks for the help. Just one other thing. Let me know if declaring a class friend of another, I respect the constraints of encapsulation and OOP? – Fab Apr 05 '15 at 00:43
  • Yes, definitely. OOP is also about defining complete, minimal and functional public interface for every class. But sometimes, another class may require access to few or more private members. Adding a lot of new function just to satisfy other class' implementation logic would be a very bad design. – Mateusz Grzejek Apr 05 '15 at 11:00