8

I'm currently reviving an old homework assignment, where I'm writing a program that among other functions, involves finding the shortest path in a graph using Dijkstra's algorithm.

I think I've got it right for the most part, but I keep getting NullPointerException at line 58 when executing if(currentNode.getAktuell()).

I've been trying several solutions back and forth but can't seem to figure out what is wrong but prioQueue.poll(); returns null when the queue is empty. I've tried to handle that last currentNode that eventually turns into null but have not been able to find a working solution, so I'm starting to think that I've missed out on something here.

I would really appreciate it if someone familiar with dijkstras algorithm could help me out here. There's probably a better solution to the algorithm but I only want help with finding out what is wrong with the one I've written, and not "the answer" using someone else's algorithm.

public static List<String> shortestPath(Graph<String> graph, String från, String till){

    //if(!pathExists(graph, från, till))
    //return null;

    PriorityQueue<DjikstraObjekt<String>> prioQueue = new PriorityQueue<DjikstraObjekt<String>>();
    LinkedHashMap<String, DjikstraObjekt<String>> samling = new LinkedHashMap<String, DjikstraObjekt<String>>();

    for(String bla : graph.getNodes())
        samling.put(bla, new DjikstraObjekt<String>(bla, Integer.MAX_VALUE, null, false));
    samling.get(från).updateVikt(0);
    prioQueue.add(samling.get(från));

    while(!samling.get(till).getAktuell())
    {

        DjikstraObjekt<String> currentNode = prioQueue.poll();
        if(currentNode==null)
            break;
        if(currentNode.getAktuell())
            continue;


        currentNode.aktuellNod();

        for(ListEdge<String> edge : graph.getEdgesFrom(currentNode.getNode()))
        {
            System.out.println("get edges from");
            int nyVikt = edge.getVikt() + currentNode.getVikt();
            DjikstraObjekt<String> toNode = samling.get(edge.getDest());
            if(!toNode.getAktuell() && nyVikt < toNode.getVikt()) {
                toNode.updateVikt(nyVikt);
                toNode.setFrån(currentNode.getNode());
                prioQueue.add(toNode);
            }
        }

    }       

    List<String> djikstaList = new ArrayList<String>();
    for(int i=0;i<samling.size();i++){
        if(samling.get(i).getNode()!=från){
            System.out.println(samling.get(i).getNode());
            djikstaList.add(samling.get(i).getNode());
        }       
    }

    return djikstaList;
}


public class DjikstraObjekt<E> implements Comparable<DjikstraObjekt<E>> {
    private E nod;
    private int vikt;
    private E frånNod;
    private boolean aktuellNod=false;

    public DjikstraObjekt(E nod, int vikt, E frånNod, boolean aktuellNod){

        this.nod=nod;
        this.vikt=vikt;
        this.frånNod=frånNod;
        this.aktuellNod=aktuellNod;

    }
    public E getNode() {
        return nod;
    }
    public void updateVikt(int nyvikt){
        vikt=nyvikt;
    }
    public int getVikt() {
        return vikt;
    }
    public boolean getAktuell() {
        return aktuellNod;
    }
    public void aktuellNod(){
        aktuellNod=true;
    }
    public void setFrån(E från)
    {
        frånNod = från;
    }
    public int compareTo(DjikstraObjekt<E> other) {
        return getVikt() - other.getVikt();
    }
}

Heres my listEdge class:

public class ListEdge<E> {

    private E dest;
    private String namn;
    private Integer vikt;


    public ListEdge(E dest, String namn, Integer vikt){
        this.dest=dest;
        this.namn=namn;
        this.vikt=vikt;

    }

    public E getDest(){
        return dest;
    }
    public void ändraVikt(Integer nyVikt){
        if(vikt<0)
            throw new IllegalArgumentException();
        vikt=nyVikt;

        }
    public String getNamn(){
        return namn;
    }
     public int compareTo(ListEdge other) {
         return this.vikt.compareTo(other.getVikt());
 }

    public int getVikt(){
        return vikt;
    }
    public String toString(){
        return "till " + dest + " med " + namn +" "+ vikt;
    }
}

These should be the relevent methods from my ListGraph class:

public List<E> getNodes(){
    List<E> temp = new ArrayList<E>();
    for(E test : noder.keySet()){
        temp.add(test);

    }
return temp;
}

public List<ListEdge<E>> getEdgesFrom(E nod) {
        List<ListEdge<E>> temp = new ArrayList<ListEdge<E>>();
        if(noder.containsKey(nod)){
            try{
                for(Map.Entry<E, List<ListEdge<E>>> test : noder.entrySet()){
                    if(test.getKey().equals(nod)){
                        System.out.println(nod+" "+test.getKey());
                        for(ListEdge<E> e: test.getValue()){
                            temp.add(e);
                    }
                }
            }
        }
            catch(NoSuchElementException E){

            }

        }
        return temp;
    }
Bill the Lizard
  • 398,270
  • 210
  • 566
  • 880
John Snow
  • 5,214
  • 4
  • 37
  • 44
  • 9
    For non-Scandinavians, "vikt" means "weight", "från" means "from", and "aktuell" means "current". – Aasmund Eldhuset Jul 05 '11 at 15:38
  • yeah sorry about that. I have a bad habbit of mixing variables/methods names with english and swedish :) – John Snow Jul 05 '11 at 15:42
  • Please gives us your code of DijkstraObject, too, or the complete stacktrace. As you check `currentNode` for `null` just before that line, the `NullPointerException` has to originate from the `getAktuell()` method. – Frank Jul 06 '11 at 06:22
  • I did include the DijkstraObject class at the bottom of the page – John Snow Jul 06 '11 at 09:24
  • 1
    If you provide the Graph and ListEdge classes, it will be easier to find the bug. If you didn't solve it yet, please submit that info. I'm curious about this :) – Leandro Aug 01 '11 at 22:00
  • Hi! I've added the relevent methods to my mainpost, which would be my ListEdge class, and the relevent methods from the graph. Please let me know if you can find out what's wrong :) – John Snow Aug 05 '11 at 13:32

3 Answers3

3

I couldn't reconstruct the NullPointerException you told us about. As Leandro pointed out, the problem might lay with your implementation of ListEdge and Graph.

I did an implementation of both classes myself to test your code.

The only problem I could find was in the end where you create the result list:

for(int i=0;i<samling.size();i++){
        if(samling.get(i).getNode()!=från){

This will always Result in a NullPointerException because get() expects a key and in your case that's a String, not an int. To iterate over the Map use something like

List<String> djikstaList = new ArrayList<String>();
for(String key : samling.keySet()){
    if(samling.get(key).getNode()!=från){
        System.out.println(samling.get(key).getNode());
        djikstaList.add(samling.get(key).getNode());
    }       
}

Furthermore, i assume you wan't to return the actual path from from to to so you would need to add a getter getFrån() to DijkstraObjekt and then build up the list like this:

   String fromNode = samling.get(to).getNode();
   djikstaList.add(to);
   while(fromNode != from){   
       fromNode = samling.get(fromNode).getFrån();
       djikstaList.add(fromNode);
   }

After this the List will contain the complete path (including Start and End node) in reverse order.

If wanted, I can post all of my classes I used for testing/debugging.

Cheers tannerli

tannerli
  • 594
  • 9
  • 33
  • Hi! I've edited my mainpost to add the missing metods from my Graph. Not at home right now so can't try out your answer yet but i'll let you know if im able to solve the problem with your answer – John Snow Aug 05 '11 at 13:30
  • I found nothing obviously wrong with these 2 classes you added (altough it would have been nice to have the whole ListGraph class), I think the problem stuck with the part I mentioned, because I had to change nothing at the logic of the algorithm itself for me to make it work. – tannerli Aug 08 '11 at 08:20
  • Thanks for your help. Im on vacation right now but ill try your suggestion and mark it as solved if ill get it to work as soon as I come home which will be in two weeks :) – John Snow Aug 08 '11 at 11:19
  • I got the algorithm to work. Was building the list in the wrong way as you said. I also added from which node you came, and in what way to the djikstraObject class. Thanks alot, you're a lifesaver :) – John Snow Aug 24 '11 at 15:42
  • Always glad to help, i had to study for my "datastructures and algorithms"-exam anyway :) – tannerli Aug 25 '11 at 14:23
0

I think this might be a problem:

//...
samling.put(bla, new DjikstraObjekt<String>(bla, Integer.MAX_VALUE, null, false));
samling.get(från).updateVikt(0);

EDIT:

Sorry I thought the {} is there. Everything is ok there. I will keep looking.

Zoltan Balazs
  • 837
  • 5
  • 7
0

Maybe try this:

if(currentNode==null || currentNode.getAktuell() == null)
         break;        
if(currentNode.getAktuell())            
        continue;
Jason Rae
  • 2,583
  • 5
  • 25
  • 34
  • I tried that earlier but it will give me a nullpointer later on in the code. Think i've done something else wrong here but can't figure out what it is – John Snow Jul 06 '11 at 10:05