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!

Improve this question
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 =( 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

Reset to default
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.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

I've changed both iterators to const_iterators and redefined getEdges and getEndpoints to const functions, no change in compiler error.
@Jared I updated the answer. I think these changes should calm down the compiler.
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?
That last comment was before the (*sit)->getEndpoints() suggestion. That solved my problem right up, thanks!
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()

Improve this answer

2 Comments

(*it)->foo() did the trick! What exactly do the parens do? Just tell the compiler to treat the dereferenced iterator as an object?
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.
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.

Improve this answer

1 Comment

I've changed both iterators to const_iterator and the error is exactly the same.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.