Implementation of Dijkstra's algorithm

Please pick my code apart and give me some feedback on how I could make it better or more simple.

```
final class Edge {
private final Node node1, node2;
private final int distance;

public Edge (Node node1, Node node2, int distance) {
this.node1 = node1;
this.node2 = node2;
this.distance = distance;
}

public Node getAdjacentNode (Node node) {
return node.getValue() != node1.getValue() ? node1 : node2;
}

public int getDistance() {
return distance;
}
}

class Node {
private final int v;
private int distance = Integer.MAX_VALUE;

public Node (int v) {
this.v = v;
}

public int getValue() {
return v;
}
public int getDistance() {
return distance;
}

public void setDistance(int distance) {
this.distance = distance;
}

@Override
public boolean equals(Object o) {
Node node = (Node) o;
return node.getValue() == v;
}

@Override
public int hashCode() {
return v;
}
}

class Graphs {

private final Map> map;
private final int numberOfVertices;

Graphs(int numberOfVertices) {
if (numberOfVertices >();
}

public void addEdge (Node node1, Node node2, int distance) {
// necessary to throw null ptr exceptions explicitly since, null can get propagated and saved in edge
if (node1 == null || node2 == null) {
throw new NullPointerException("Either of the 2 nodes is null.");
}
if (distance l = map.get(node);
l.add(edge);
} else {
List l = new ArrayList();
l.add(edge);
map.put(node, (ArrayList) l);
}
}

public List getAdj(Node node) {
return map.get(node);
}

public Map> getGraph() {
return map;
}

public int getNumVertices() {
return numberOfVertices;
}
}

public class Dijkstra {

private final Graphs

-
In your Edge class getAdjacentNode() returns node1 or node2 regardless whether the parameter is one of the two. You should consider throwing an exception (IllegalStateEeption maybe) if the parameter is neither of them.

-
Your Graphs class should be Graph - an instance of that class represents a single graph and not multiple ones, doesn't it?

-
What is the point of the numberOfVertices member? It's not used for anything. You should remove it.

-
You use the distance member of the node to store transient values during the computation in Dijkstra. It's never good to intermingle data representation and iteration over the data. Keep the transient data you need for Dijkstra separate from the nodes and edges. It might incur some extra memory overhead but it's usually worth it. For example if your algorithm fails then it might leave the graph in an inconsistent state. Also you have to reset the graph when you want to re-compute.

-
This code gets the node with the given value:

for (Entry> entry :  graph.getGraph().entrySet()) {
    Node currNode = entry.getKey();
    if (currNode.getValue() == source) {
        currNode.setDistance(0);
        queue.add(currNode);
    } 
}

I would add this as a method to Graph like getNode(int value). It makes sense to be able to ask a graph for a specific node.

-
You should not have to care how the graph internally represents its nodes and edges. So instead of having a method getGraph which just dumps the internally used data structure onto the user have a define interface All you need is either a specific node (see point 5) or all nodes. So add another method getAllNodes() which returns all nodes.

Alternatively consider adding a custom iterator which allows users to traverse all the nodes of the graph without having to care about the internal representation.

The reasoning behind that is: If you ever want to change your internal representation then you either have to convert it to the exposed data structure (means you might have to copy around a lot of data) or you have to change all code using it. Neither of them is particularly nice.

StackExchange Code Review Q#32354, answer score: 8