0

I am new to advanced algorithms, so please bear with me. I am currently trying to get Dijkstra's algorithm to work, and have spend 2 days trying to figure this out. I also read the pseudo-code on Wikipedia and got this far. I want to get the shortest distance between two vertices. In the below sample, I keep getting the wrong distance. Please help?

Sample graph setup is as follow:

Graph graph = new Graph();
graph.Vertices.Add(new Vertex("A"));
graph.Vertices.Add(new Vertex("B"));
graph.Vertices.Add(new Vertex("C"));
graph.Vertices.Add(new Vertex("D"));
graph.Vertices.Add(new Vertex("E"));

graph.Edges.Add(new Edge
                    {
                        From = graph.Vertices.FindVertexByName("A"),
                        To = graph.Vertices.FindVertexByName("B"),
                        Weight = 5
                    });

graph.Edges.Add(new Edge
                    {
                        From = graph.Vertices.FindVertexByName("B"),
                        To = graph.Vertices.FindVertexByName("C"),
                        Weight = 4
                    });

graph.Edges.Add(new Edge
                    {
                        From = graph.Vertices.FindVertexByName("C"),
                        To = graph.Vertices.FindVertexByName("D"),
                        Weight = 8
                    });


graph.Edges.Add(new Edge
                    {
                        From = graph.Vertices.FindVertexByName("D"),
                        To = graph.Vertices.FindVertexByName("C"),
                        Weight = 8
                    });

//graph is passed as param with source and dest vertices
public int Run(Graph graph, Vertex source, Vertex destvertex)
{
    Vertex current = source;
    List<Vertex> queue = new List<Vertex>();

    foreach (var vertex in graph.Vertices)
    {
        vertex.Weight = int.MaxValue;
        vertex.PreviousVertex = null;
        vertex.State = VertexStates.UnVisited;
        queue.Add(vertex);
    }

    current = graph.Vertices.FindVertexByName(current.Name);
    current.Weight = 0;
    queue.Add(current);
    while (queue.Count > 0)
    {
        Vertex minDistance = queue.OrderBy(o => o.Weight).FirstOrDefault();
        queue.Remove(minDistance);

        if (current.Weight == int.MaxValue)
        {
            break;
        }

        minDistance.Neighbors = graph.GetVerTextNeigbours(current);

        foreach (Vertex neighbour in minDistance.Neighbors)
        {
            Edge edge = graph.Edges.FindEdgeByStartingAndEndVertex(minDistance, neighbour);
            int dist = minDistance.Weight + (edge.Weight);
            if (dist < neighbour.Weight)
            {
                //from this point onwards i get stuck
                neighbour.Weight = dist;
                neighbour.PreviousVertex = minDistance;
                queue.Remove(neighbour);
                queueVisited.Enqueue(neighbor);
            }
        }
        minDistance.State = VertexStates.Visited;
    }

      //here i want to record all node that was visited
     while (queueVisited.Count > 0)
    {
        Vertex temp = queueVisited.Dequeue();
        count += temp.Neighbors.Sum(x => x.Weight);
    }

    return count;
}
user353gre3
  • 2,747
  • 4
  • 24
  • 27
user3298441
  • 91
  • 1
  • 4

1 Answers1

0

There are several immediate issues with the above code.

  1. The current variable is never re-assigned, nor is the object mutatd, so graph.GetVerTextNeigbours(current) always returning the same set.

    As a result, this will never even find the target vertex if more than one edge must be traversed.

  2. The neighbor node is removed from the queue via queue.Remove(neighbour). This is incorrect and can prevent a node/vertex from being [re]explored. Instead, it should just have the weight updated in the queue.

    In this case the "decrease-key v in Q" is the psuedo-code results in a NO-OP in the implementation due object-mutation and not using a priority queue (i.e. sorted) structure.

  3. The visited queue, as per queueVisited.Enqueue(neighbor) and the later "sum of all neighbor weights of all visited nodes", is incorrect.

    After the algorithm runs, the cost (Weight) of any node/vertex that was explored is the cost of the shortest path to that vertex. Thus, you can trivially ask the goal vertex what the cost is.


In addition, for the sake of the algorithm simplicity, I recommend giving up the "Graph/Vertex/Edge" layout initially created and, instead, pass in a simple Node. This Node can enumerate its neighbor Nodes, knows the weights to each neighbor, its own current weight, and the best-cost node to it.

Simply build this Node graph once, at the start, then pass the starting Node. This eliminates (or greatly simplifies) calls like GetVerTextNeighbors and FindEdgeByStartingAndEndVertex, and strange/unnecessary side-effects like assigning Neighbors each time the algorithm visits the vertex.

user2864740
  • 60,010
  • 15
  • 145
  • 220