1

I'm working on a program that's supposed to represent a graph. My issue is in my printAdjacencyList function. Basically, I have a Graph ADT that has a member variable "nodes", which is a map of the nodes of that graph. Each Node has a set of Edge* to the edges it is connected to. I'm trying to iterate across each node in the graph and each edge of a node.

void MyGraph::printAdjacencyList() {
std::map<std::string, MyNode*>::iterator mit;
std::set<MyEdge*>::iterator sit;

for (mit = nodes.begin(); mit != nodes.end(); mit++ ) {
    std::cout << mit->first << ": {";
    const std::set<MyEdge*> edges = mit->second->getEdges();
    for (sit = edges.begin(); sit != edges.end(); sit++) {
        std::pair<MyNode*, MyNode*> edgeNodes = *sit->getEndpoints();
    }
}
std::cout << " }" << std::endl;
}

getEdges is declared as:

const std::set<MyEdge*>& getEdges() { return edges; };

and get Endpoints is declared as:

const std::pair<MyNode*, MyNode*>& getEndpoints() { return nodes; };

The compiler error I'm getting is:

MyGraph.cpp:63: error: request for member `getEndpoints' in 
`*(&sit)->std::_Rb_tree_const_iterator<_Tp>::operator->
[with _Tp = MyEdge*]()', which is of non-class type `MyEdge* const'

MyGraph.cpp:63: warning: unused variable 'edgeNodes'

I have figured out that this probably means I'm misusing const somewhere, but I can't figure out where for the life of me. Any information would be appreciated. Thanks!

6
  • Unless you are doing this project for homework or for fun, I would suggest you just use the boost library graph representation. Commented May 8, 2010 at 22:56
  • Yeah, this is homework for a class, so no boost niceness =(
    – Jared
    Commented May 8, 2010 at 23:03
  • @Jared good luck with that, then :) also, you can consider tagging these kind of question with the 'homework' tag, so people will know. Commented May 8, 2010 at 23:07
  • @Jared @Amir not to worry. If you don't use the BGL you can be grateful for avoiding a lot of headache. Moving from your own mocked graph functionality to the BGL is like moving from stdio.h to iostream. You'll probably be grateful for that one day, but until then the number of cursewords coming out of your mouth will not put an English football fan to shame. Commented May 8, 2010 at 23:14
  • You stated that nodes is a map, however in getEndpoints you return it as an std::pair. How is this right? Commented May 8, 2010 at 23:14

3 Answers 3

5

Try changing sit to a const_iterator. Change mit to a const_iterator too while you're at it. Also, getEdges() and getEndpoints() should be const functions. Lastly, because operator->() has a higher precedence than the unary operator*(), you probably want to say edgeNodes = (*sit)->getEndPoints() inside the inner-loop.

Not as much of a problem but you should consider having the iterator instances as local to the loops.

4
  • 1
    I've changed both iterators to const_iterators and redefined getEdges and getEndpoints to const functions, no change in compiler error.
    – Jared
    Commented May 8, 2010 at 23:15
  • @Jared I updated the answer. I think these changes should calm down the compiler. Commented May 8, 2010 at 23:34
  • 1
    If you still get these errors after these, what happens if you change the return types of those accessors to a return-by-value, instead of return by const-reference? Commented May 8, 2010 at 23:47
  • That last comment was before the (*sit)->getEndpoints() suggestion. That solved my problem right up, thanks!
    – Jared
    Commented May 8, 2010 at 23:58
2

It's hard to tell anything definite without something compilable, but

const std::set<MyEdge*>& getEdges() { return edges; };

and

const std::pair<MyNode*, MyNode*>& getEndpoints() { return nodes; };

should technically be const methods since they don't modify the class and return a const reference.

const std::set<MyEdge*>& getEdges() const { return edges; };
const std::pair<MyNode*, MyNode*>& getEndpoints() const { return nodes; };

This in combination with const_iterator might solve your constness problems.


However, your particular error might be that *it->foo() = *(it->foo())is different from (*it)->foo()

2
  • (*it)->foo() did the trick! What exactly do the parens do? Just tell the compiler to treat the dereferenced iterator as an object?
    – Jared
    Commented May 8, 2010 at 23:26
  • They change the precedence. The -> operator has more precedence than the * operator, so it is evaluated first. But you need the * to be evaluated first, so you enclose it with the operand inside the parenthesis. Commented May 8, 2010 at 23:54
1

s/std::set<MyEdge*>::iterator sit;/std::set<MyEdge*>::const_iterator sit;/

and ditto for mit. In other words, const_iterator is what you need here.

1
  • 1
    I've changed both iterators to const_iterator and the error is exactly the same.
    – Jared
    Commented May 8, 2010 at 23:05

Not the answer you're looking for? Browse other questions tagged or ask your own question.