Search code examples
c++algorithmgraphqueuevertex

Queue Problem while modeling a shortest path algorithm


I have a queue problem. Modeling a graph, I'm doing a shortest path algorithm in C++.

In my while (!q.empty()) the front vertex* gets changed when I return to this statement.

Can you figure out why?

int MyMatrix::searchBreadth(MyVertex* from,MyVertex* to)
{
queue<MyVertex*> q;  
path=INFINITY;

from->visit();  
from->setDistance(0);  
q.push(from);  

//here q.front()'s attributes get changed when returning from the for-loop  
while(!q.empty())
{  
    MyVertex* v=q.front();  
    q.pop();  
    int k=v->getDistance();  
    vector<MyVertex> nb=getNeighbours(*v);  
    for(int i=0;i<nb.size();i++)  
    {  
        if(nb[i].getDistance()==INFINITY)
        {  
            nb[i].setDistance(k+1);  
            q.push(&nb[i]);  
        }

        if((nb[i].getName().compare(to->getName())==0)
           && !nb[i].isVisited())
        {
            //path found  
            int j=nb[i].getDistance();  
            if(j<path) path=j;  
        }  

        nb[i].visit();  
     }  
}  
return path;  

}   

here comes getNeighbours()

vector<MyVertex> MyMatrix::getNeighbours(MyVertex &v)
{  
    int index=0;  
    for(int l=0; l<stations.size(); l++ )
    {  
        if(stations[l].getName().compare(v.getName())==0)index=l;  
    }

    vector<MyVertex> out;  
    for(int k=0;k<matrixSize;k++)
    {  
        if(matrix[index][k].getName().compare("null")!=0)
        {  
            out.push_back(matrix[index][k].getTo());  
        }  
    }  

    return out;
}

Solution

  • Your problem is subtle, but related to q.push(&nb[i]). What you're doing is adding a pointer to a location in a vector, which is not conceptually the same as adding a pointer to a MyVertex object. The vector of neighbors contains the MyVertex objects "by value" (if that helps in your understanding of the problem).

    A look at nb in memory may help:

            0         1                   I
    nb [MyVertex0|MyVertex1|   ...   |MyVertexI]
                 +---------+
                      | (Notice it is NOT pointing to MyVertex1!)
    &nb[1]------------+
    

    When you push &nb[1] you're pushing the address nb + (1 * sizeof(MyVertex)). nb is declared on the stack, so that address is going to be somewhere on the stack.

    So when your for-loop comes back around, nb gets refreshed (so to speak) and new data is added. However, your queue q contains addresses into nb that are no longer valid!

    Simply put: your queue is referencing a LOCATION in the vector, not the DATA in the vector.

    If you want to keep your method as-is, this means getNeighbors needs to change to return a vector of MyVertex*.


    You should simply edit BreadthFirstSearch to take two MyVertex&, rather than pointers. You would then change q to be a queue<MyVertex>, v to MyVertex, and finally you should change q.push(&nb[i]) to just q.push(nb[i]).