YAKOVM
YAKOVM

Reputation: 10163

Iterator set in insert function

I am implementing an iterator for Graph class

class iterator_node {
    private:
        Graph* gr_;
        Node* curNode_;
    public:
        iterator_node() : gr_(NULL),curNode_(NULL) {}
        iterator_node(Graph& ddg) : gr_(&ddg),curNode_(NULL) {}
        ~iterator_node() {}
}

As well I have a Graph class for which I should implement insert function

class Graph {
private:
//....
map<NodeID,Node*> nodes_;
public:
pair<iterator_node,bool> insert(const NodeID nodeId,const NodeLabel nodeLabel);
}
pair<iterator_node,bool> Graph::insert(const NodeID nodeId,const NodeLabel nodeLabel)
{
    DDG::iterator_node itNode(*this);
    pair<Graph::iterator_node,bool> res(itNode,false);
    Node* node = new Node(nodeId,false,nodeLabel);
    pair<map<NodeID,Node*>::iterator,bool> nodeInsRet  = nodes_.insert(pair<NodeID,Node*> (nodeId,node));
    if (nodeInsRet.second == false) {
        std::cout << "Node with nodeID=" << nodeId << " already exists \n";
        delete node;
    }
    else {
        res.second = true;
       //!!Question!!!!
       //should update curNode_; in res`s iterator (res.first)
       //what is a relevan correct way to do it?
    }

    return res;
}

Upvotes: 0

Views: 88

Answers (1)

Jonathan Wakely
Jonathan Wakely

Reputation: 171501

Your iterator can't iterate unless it can move from one element to the next. With your current code incrementing the iterator_node you'd have to search the entire Graph for the current Node and then find the next one ... which would be incredibly slow.

Either you need to link each Node to its neighbours, or your iterator_node should store a map<NodeID,Node*>::iterator instead of a Node*

Upvotes: 2

Related Questions