Skip to content

Commit 863d439

Browse files
author
Justin Wetherell
committed
Found a bug in dijkstra's algo, also did a bit of cleanup in the graph classes
1 parent d54bc15 commit 863d439

File tree

7 files changed

+141
-141
lines changed

7 files changed

+141
-141
lines changed

‎src/com/jwetherell/algorithms/graph/BellmanFord.java‎

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,18 +18,20 @@
1818
*/
1919
public class BellmanFord {
2020

21-
private static Map<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>> costs = null;
22-
private static Map<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>> paths = null;
23-
2421
private BellmanFord() { }
2522

26-
public static Map<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>> getShortestPaths(Graph<Integer> g, Graph.Vertex<Integer> start) {
27-
getShortestPath(g, start, null);
28-
Map<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>> map = new HashMap<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>>();
23+
public static Map<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>> getShortestPaths(Graph<Integer> graph, Graph.Vertex<Integer> start) {
24+
final Map<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>> paths = new HashMap<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>>();
25+
final Map<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>> costs = new HashMap<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>>();
26+
27+
getShortestPath(graph, start, null, paths, costs);
28+
29+
final Map<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>> map = new HashMap<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>>();
2930
for (Graph.CostVertexPair<Integer> pair : costs.values()) {
30-
int cost = pair.getCost();
31-
Graph.Vertex<Integer> vertex = pair.getVertex();
32-
Set<Graph.Edge<Integer>> path = paths.get(vertex);
31+
final int cost = pair.getCost();
32+
final Graph.Vertex<Integer> vertex = pair.getVertex();
33+
final Set<Graph.Edge<Integer>> path = paths.get(vertex);
34+
3335
map.put(vertex, new Graph.CostPathPair<Integer>(cost, path));
3436
}
3537
return map;
@@ -39,15 +41,21 @@ public static Graph.CostPathPair<Integer> getShortestPath(Graph<Integer> graph,
3941
if (graph == null)
4042
throw (new NullPointerException("Graph must be non-NULL."));
4143

42-
// Reset variables
43-
costs = null;
44-
paths = null;
44+
final Map<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>> paths = new HashMap<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>>();
45+
final Map<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>> costs = new HashMap<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>>();
46+
return getShortestPath(graph, start, end, paths, costs);
47+
}
48+
49+
private static Graph.CostPathPair<Integer> getShortestPath(Graph<Integer> graph,
50+
Graph.Vertex<Integer> start, Graph.Vertex<Integer> end,
51+
Map<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>> paths,
52+
Map<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>> costs) {
53+
if (graph == null)
54+
throw (new NullPointerException("Graph must be non-NULL."));
4555

46-
paths = new HashMap<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>>();
4756
for (Graph.Vertex<Integer> v : graph.getVerticies())
4857
paths.put(v, new LinkedHashSet<Graph.Edge<Integer>>());
4958

50-
costs = new HashMap<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>>();
5159
for (Graph.Vertex<Integer> v : graph.getVerticies())
5260
if (v.equals(start))
5361
costs.put(v, new Graph.CostVertexPair<Integer>(0, v));

‎src/com/jwetherell/algorithms/graph/ConnectedComponents.java‎

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,29 @@ public class ConnectedComponents {
2222

2323
private ConnectedComponents() { }
2424

25+
/**
26+
* Finds the connected components subsets of the Graph.
27+
*
28+
* @param g Graph to find connected components.
29+
* @return List of connected components in the Graph.
30+
*/
31+
public static final <T extends Comparable<T>> List<List<Vertex<T>>> getConnectedComponents(Graph<T> graph) {
32+
if (graph == null)
33+
throw new IllegalArgumentException("Graph is NULL.");
34+
35+
if (graph.getType() != Graph.TYPE.DIRECTED)
36+
throw new IllegalArgumentException("Cannot perform a connected components search on a non-directed graph. graph type = "+graph.getType());
37+
38+
final Map<Vertex<T>,Integer> map = new HashMap<Vertex<T>,Integer>();
39+
final List<List<Vertex<T>>> list = new ArrayList<List<Vertex<T>>>();
40+
41+
int c = 0;
42+
for (Vertex<T> v : graph.getVerticies())
43+
if (map.get(v) == null)
44+
visit(map, list, v, c++);
45+
return list;
46+
}
47+
2548
private static final <T extends Comparable<T>> void visit(Map<Vertex<T>,Integer> map, List<List<Vertex<T>>> list, Vertex<T> v, int c) {
2649
map.put(v, c);
2750

@@ -47,27 +70,4 @@ private static final <T extends Comparable<T>> void visit(Map<Vertex<T>,Integer>
4770
}
4871
}
4972
}
50-
51-
/**
52-
* Finds the connected components subsets of the Graph.
53-
*
54-
* @param g Graph to find connected components.
55-
* @return List of connected components in the Graph.
56-
*/
57-
public static final <T extends Comparable<T>> List<List<Vertex<T>>> getConnectedComponents(Graph<T> graph) {
58-
if (graph == null)
59-
throw new IllegalArgumentException("Graph is NULL.");
60-
61-
if (graph.getType() != Graph.TYPE.DIRECTED)
62-
throw new IllegalArgumentException("Cannot perform a connected components search on a non-directed graph. graph type = "+graph.getType());
63-
64-
final Map<Vertex<T>,Integer> map = new HashMap<Vertex<T>,Integer>();
65-
final List<List<Vertex<T>>> list = new ArrayList<List<Vertex<T>>>();
66-
67-
int c = 0;
68-
for (Vertex<T> v : graph.getVerticies())
69-
if (map.get(v) == null)
70-
visit(map, list, v, c++);
71-
return list;
72-
}
7373
}

‎src/com/jwetherell/algorithms/graph/CycleDetection.java‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@
1313
*/
1414
public class CycleDetection {
1515

16-
private static Set<Graph.Vertex<Integer>> visitedVerticies = new HashSet<Graph.Vertex<Integer>>();
17-
private static Set<Graph.Edge<Integer>> visitedEdges = new HashSet<Graph.Edge<Integer>>();
18-
1916
private CycleDetection() { }
2017

2118
/**
@@ -31,32 +28,35 @@ public static boolean detect(Graph<Integer> graph) {
3128
if (graph.getType() != Graph.TYPE.UNDIRECTED)
3229
throw new IllegalArgumentException("Graph is needs to be Undirected.");
3330

34-
visitedVerticies.clear();
35-
visitedEdges.clear();
36-
List<Graph.Vertex<Integer>> verticies = graph.getVerticies();
31+
final Set<Graph.Vertex<Integer>> visitedVerticies = new HashSet<Graph.Vertex<Integer>>();
32+
final Set<Graph.Edge<Integer>> visitedEdges = new HashSet<Graph.Edge<Integer>>();
33+
34+
final List<Graph.Vertex<Integer>> verticies = graph.getVerticies();
3735
if (verticies == null || verticies.size() == 0)
3836
return false;
3937

4038
// Select the zero-ith element as the root
41-
Graph.Vertex<Integer> root = verticies.get(0);
42-
return depthFirstSearch(root);
39+
final Graph.Vertex<Integer> root = verticies.get(0);
40+
return depthFirstSearch(root, visitedVerticies, visitedEdges);
4341
}
4442

45-
private static final boolean depthFirstSearch(Graph.Vertex<Integer> vertex) {
43+
private static final boolean depthFirstSearch(Graph.Vertex<Integer> vertex, Set<Graph.Vertex<Integer>> visitedVerticies, Set<Graph.Edge<Integer>> visitedEdges) {
4644
if (!visitedVerticies.contains(vertex)) {
4745
// Not visited
4846
visitedVerticies.add(vertex);
4947

50-
List<Graph.Edge<Integer>> edges = vertex.getEdges();
48+
final List<Graph.Edge<Integer>> edges = vertex.getEdges();
5149
if (edges != null) {
5250
for (Graph.Edge<Integer> edge : edges) {
53-
Graph.Vertex<Integer> to = edge.getToVertex();
51+
final Graph.Vertex<Integer> to = edge.getToVertex();
5452
boolean result = false;
5553
if (to != null && !visitedEdges.contains(edge)) {
5654
visitedEdges.add(edge);
57-
Graph.Edge<Integer> recip = new Graph.Edge<Integer>(edge.getCost(), edge.getToVertex(), edge.getFromVertex());
55+
56+
final Graph.Edge<Integer> recip = new Graph.Edge<Integer>(edge.getCost(), edge.getToVertex(), edge.getFromVertex());
5857
visitedEdges.add(recip);
59-
result = depthFirstSearch(to);
58+
59+
result = depthFirstSearch(to, visitedVerticies, visitedEdges);
6060
}
6161
if (result == true)
6262
return result;

‎src/com/jwetherell/algorithms/graph/Dijkstra.java‎

Lines changed: 39 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,12 @@
2020
*/
2121
public class Dijkstra {
2222

23-
private static Map<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>> costs = null;
24-
private static Map<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>> paths = null;
25-
private static Queue<Graph.CostVertexPair<Integer>> unvisited = null;
26-
2723
private Dijkstra() { }
2824

29-
public static Map<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>> getShortestPaths(Graph<Integer> g, Graph.Vertex<Integer> start) {
30-
getShortestPath(g, start, null);
25+
public static Map<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>> getShortestPaths(Graph<Integer> graph, Graph.Vertex<Integer> start) {
26+
final Map<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>> paths = new HashMap<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>>();
27+
final Map<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>> costs = new HashMap<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>>();
28+
getShortestPath(graph, start, null, paths, costs);
3129
Map<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>> map = new HashMap<Graph.Vertex<Integer>, Graph.CostPathPair<Integer>>();
3230
for (Graph.CostVertexPair<Integer> pair : costs.values()) {
3331
int cost = pair.getCost();
@@ -42,57 +40,69 @@ public static Graph.CostPathPair<Integer> getShortestPath(Graph<Integer> graph,
4240
if (graph == null)
4341
throw (new NullPointerException("Graph must be non-NULL."));
4442

45-
// Reset variables
46-
costs = null;
47-
paths = null;
48-
unvisited = null;
43+
// Dijkstra's algorithm only works on positive cost graphs
44+
boolean hasNegativeEdge = checkForNegativeEdges(graph.getVerticies());
45+
if (hasNegativeEdge)
46+
throw (new IllegalArgumentException("Negative cost Edges are not allowed."));
47+
48+
final Map<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>> paths = new HashMap<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>>();
49+
final Map<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>> costs = new HashMap<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>>();
50+
return getShortestPath(graph, start, end, paths, costs);
51+
}
52+
53+
private static Graph.CostPathPair<Integer> getShortestPath(Graph<Integer> graph,
54+
Graph.Vertex<Integer> start, Graph.Vertex<Integer> end,
55+
Map<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>> paths,
56+
Map<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>> costs) {
57+
if (graph == null)
58+
throw (new NullPointerException("Graph must be non-NULL."));
4959

5060
// Dijkstra's algorithm only works on positive cost graphs
5161
boolean hasNegativeEdge = checkForNegativeEdges(graph.getVerticies());
5262
if (hasNegativeEdge)
5363
throw (new IllegalArgumentException("Negative cost Edges are not allowed."));
5464

55-
paths = new HashMap<Graph.Vertex<Integer>, Set<Graph.Edge<Integer>>>();
5665
for (Graph.Vertex<Integer> v : graph.getVerticies())
5766
paths.put(v, new LinkedHashSet<Graph.Edge<Integer>>());
5867

59-
costs = new HashMap<Graph.Vertex<Integer>, Graph.CostVertexPair<Integer>>();
6068
for (Graph.Vertex<Integer> v : graph.getVerticies()) {
6169
if (v.equals(start))
6270
costs.put(v, new Graph.CostVertexPair<Integer>(0, v));
6371
else
6472
costs.put(v, new Graph.CostVertexPair<Integer>(Integer.MAX_VALUE, v));
6573
}
6674

67-
unvisited = new PriorityQueue<Graph.CostVertexPair<Integer>>();
68-
unvisited.addAll(costs.values()); // Shallow copy which is O(n log n)
75+
final Queue<Graph.CostVertexPair<Integer>> unvisited = new PriorityQueue<Graph.CostVertexPair<Integer>>();
76+
unvisited.add(costs.get(start));
77+
78+
while (!unvisited.isEmpty()) {
79+
final Graph.CostVertexPair<Integer> pair = unvisited.remove();
80+
final Graph.Vertex<Integer> vertex = pair.getVertex();
6981

70-
Graph.Vertex<Integer> vertex = start;
71-
while (true) {
7282
// Compute costs from current vertex to all reachable vertices which haven't been visited
7383
for (Graph.Edge<Integer> e : vertex.getEdges()) {
74-
Graph.CostVertexPair<Integer> pair = costs.get(e.getToVertex()); // O(log n)
75-
Graph.CostVertexPair<Integer> lowestCostToThisVertex = costs.get(vertex); // O(log n)
76-
int cost = lowestCostToThisVertex.getCost() + e.getCost();
77-
if (pair.getCost() == Integer.MAX_VALUE) {
84+
final Graph.CostVertexPair<Integer> toPair = costs.get(e.getToVertex()); // O(1)
85+
final Graph.CostVertexPair<Integer> lowestCostToThisVertex = costs.get(vertex); // O(1)
86+
final int cost = lowestCostToThisVertex.getCost() + e.getCost();
87+
if (toPair.getCost() == Integer.MAX_VALUE) {
7888
// Haven't seen this vertex yet
79-
pair.setCost(cost);
8089

8190
// Need to remove the pair and re-insert, so the priority queue keeps it's invariants
82-
unvisited.remove(pair); // O(n)
83-
unvisited.add(pair); // O(log n)
91+
unvisited.remove(toPair); // O(n)
92+
toPair.setCost(cost);
93+
unvisited.add(toPair); // O(log n)
8494

8595
// Update the paths
8696
Set<Graph.Edge<Integer>> set = paths.get(e.getToVertex()); // O(log n)
8797
set.addAll(paths.get(e.getFromVertex())); // O(log n)
8898
set.add(e);
89-
} else if (cost < pair.getCost()) {
99+
} else if (cost < toPair.getCost()) {
90100
// Found a shorter path to a reachable vertex
91-
pair.setCost(cost);
92101

93102
// Need to remove the pair and re-insert, so the priority queue keeps it's invariants
94-
unvisited.remove(pair); // O(n)
95-
unvisited.add(pair); // O(log n)
103+
unvisited.remove(toPair); // O(n)
104+
toPair.setCost(cost);
105+
unvisited.add(toPair); // O(log n)
96106

97107
// Update the paths
98108
Set<Graph.Edge<Integer>> set = paths.get(e.getToVertex()); // O(log n)
@@ -106,24 +116,12 @@ public static Graph.CostPathPair<Integer> getShortestPath(Graph<Integer> graph,
106116
if (end != null && vertex.equals(end)) {
107117
// If we are looking for shortest path, we found it.
108118
break;
109-
} else if (unvisited.size() > 0) {
110-
// If there are other vertices to visit (which haven't been visited yet)
111-
Graph.CostVertexPair<Integer> pair = unvisited.remove(); // O(log n)
112-
vertex = pair.getVertex();
113-
if (pair.getCost() == Integer.MAX_VALUE) {
114-
// If the only edge left to explore has MAX_VALUE then it
115-
// cannot be reached from the starting vertex
116-
break;
117-
}
118-
} else {
119-
// No more edges to explore, we are done.
120-
break;
121119
}
122120
}
123121

124122
if (end != null) {
125-
Graph.CostVertexPair<Integer> pair = costs.get(end);
126-
Set<Graph.Edge<Integer>> set = paths.get(end);
123+
final Graph.CostVertexPair<Integer> pair = costs.get(end);
124+
final Set<Graph.Edge<Integer>> set = paths.get(end);
127125
return (new Graph.CostPathPair<Integer>(pair.getCost(), set));
128126
}
129127
return null;

‎src/com/jwetherell/algorithms/graph/FloydWarshall.java‎

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,28 +16,27 @@
1616
*/
1717
public class FloydWarshall {
1818

19-
private FloydWarshall() {
20-
}
19+
private FloydWarshall() { }
2120

2221
public static Map<Graph.Vertex<Integer>, Map<Graph.Vertex<Integer>, Integer>> getAllPairsShortestPaths(Graph<Integer> graph) {
2322
if (graph == null)
2423
throw (new NullPointerException("Graph must be non-NULL."));
2524

26-
Map<Graph.Vertex<Integer>, Map<Graph.Vertex<Integer>, Integer>> allShortestPaths = new HashMap<Graph.Vertex<Integer>, Map<Graph.Vertex<Integer>, Integer>>();
25+
final Map<Graph.Vertex<Integer>, Map<Graph.Vertex<Integer>, Integer>> allShortestPaths = new HashMap<Graph.Vertex<Integer>, Map<Graph.Vertex<Integer>, Integer>>();
2726

28-
List<Graph.Vertex<Integer>> vertices = graph.getVerticies();
29-
int[][] sums = new int[vertices.size()][vertices.size()];
27+
final List<Graph.Vertex<Integer>> vertices = graph.getVerticies();
28+
final int[][] sums = new int[vertices.size()][vertices.size()];
3029

3130
for (int i = 0; i < sums.length; i++) {
3231
for (int j = 0; j < sums[i].length; j++) {
3332
sums[i][j] = Integer.MAX_VALUE;
3433
}
3534
}
3635

37-
List<Graph.Edge<Integer>> edges = graph.getEdges();
36+
final List<Graph.Edge<Integer>> edges = graph.getEdges();
3837
for (Graph.Edge<Integer> e : edges) {
39-
int indexOfFrom = vertices.indexOf(e.getFromVertex());
40-
int indexOfTo = vertices.indexOf(e.getToVertex());
38+
final int indexOfFrom = vertices.indexOf(e.getFromVertex());
39+
final int indexOfTo = vertices.indexOf(e.getToVertex());
4140
sums[indexOfFrom][indexOfTo] = e.getCost();
4241
}
4342

@@ -47,11 +46,13 @@ public static Map<Graph.Vertex<Integer>, Map<Graph.Vertex<Integer>, Integer>> ge
4746
if (i == j) {
4847
sums[i][j] = 0;
4948
} else {
50-
int ijCost = sums[i][j];
51-
int ikCost = sums[i][k];
52-
int kjCost = sums[k][j];
53-
int summed = (ikCost != Integer.MAX_VALUE && kjCost != Integer.MAX_VALUE) ? (ikCost + kjCost)
54-
: Integer.MAX_VALUE;
49+
final int ijCost = sums[i][j];
50+
final int ikCost = sums[i][k];
51+
final int kjCost = sums[k][j];
52+
final int summed = (ikCost != Integer.MAX_VALUE && kjCost != Integer.MAX_VALUE) ?
53+
(ikCost + kjCost)
54+
:
55+
Integer.MAX_VALUE;
5556
if (ijCost > summed)
5657
sums[i][j] = summed;
5758
}
@@ -61,12 +62,14 @@ public static Map<Graph.Vertex<Integer>, Map<Graph.Vertex<Integer>, Integer>> ge
6162

6263
for (int i = 0; i < sums.length; i++) {
6364
for (int j = 0; j < sums[i].length; j++) {
64-
Graph.Vertex<Integer> from = vertices.get(i);
65-
Graph.Vertex<Integer> to = vertices.get(j);
65+
final Graph.Vertex<Integer> from = vertices.get(i);
66+
final Graph.Vertex<Integer> to = vertices.get(j);
67+
6668
Map<Graph.Vertex<Integer>, Integer> map = allShortestPaths.get(from);
6769
if (map == null)
6870
map = new HashMap<Graph.Vertex<Integer>, Integer>();
69-
int cost = sums[i][j];
71+
72+
final int cost = sums[i][j];
7073
if (cost != Integer.MAX_VALUE)
7174
map.put(to, cost);
7275
allShortestPaths.put(from, map);

0 commit comments

Comments
 (0)