HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Finding all backedges in an undirected graph

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
graphbackedgesallundirectedfinding

Problem

I have tried to implement the DFS algorithm to find all the back-edges in a graph that lead to a cycle in the graph:

```
public class CycleDetection {
private final Deque stack;
private final Graph graph;
private final Set visited;

public CycleDetection(Graph graph) {
super();
this.graph = graph;
this.stack = new ArrayDeque();
this.visited = new LinkedHashSet();
}

public static void main1(String[] args) {
Node one = new Node("A");
Node two = new Node("B");
Node three = new Node("C");
Node four = new Node("D");
Node five = new Node("E");
Node six = new Node("F");

Graph graph = new Graph();

graph.addNode(one);
graph.addNode(two);
graph.addNode(three);
graph.addNode(four);
graph.addNode(five);
graph.addNode(six);

graph.addEdge(one, two);
graph.addEdge(one, three);
graph.addEdge(one, four);
graph.addEdge(two, five);
graph.addEdge(two, six);
graph.addEdge(three, six);

graph.addEdge(two, one);
graph.addEdge(three, one);
graph.addEdge(four, one);
graph.addEdge(five, two);
graph.addEdge(six, two);
graph.addEdge(six, three);

CycleDetection detection = new CycleDetection(graph);
detection.detectCycles(six);
}

public static void main(String[] args) {
Node zero = new Node("0");
Node one = new Node("1");
Node two = new Node("2");
Node three = new Node("3");
Node four = new Node("4");
Node five = new Node("5");

Graph graph = new Graph();

graph.addNode(zero);
graph.addNode(one);
graph.addNode(two);
graph.addNode(three);
graph.addNode(four);
graph.addNode(five);

graph.addEdge(zero, one);
graph.addEdge(zero, five);

graph.addEdge(one, zero);
graph.addEdge(one, tw

Solution

Don't use fields when you don't need them

stack and visited don't need to be fields in CycleDetection.
They can be local variables in the detectCycles method:

class CycleDetection {
    private final Graph graph;

    public CycleDetection(Graph graph) {
        this.graph = graph;
    }

    public void detectCycles(Node source) {
        Deque stack = new ArrayDeque();
        Set visited = new LinkedHashSet();

        stack.push(source);

        // ...


Avoid magic string literals

The string literal "grey" appears twice.
This is error-prone, as if you want to change later,
you have to remember to make the same change everywhere.
You might also mistype it. It's better to put it in private static final String variable (a constant).

Use the diamond operator <>

As of Java 7, you don't need to specify the generic type parameters when instantiating objects when the type can be inferred from the context like here:

Deque stack = new ArrayDeque();
    Set visited = new LinkedHashSet();


You can simplify using the diamond operator <>

Deque stack = new ArrayDeque<>();
    Set visited = new LinkedHashSet<>();


Use unit tests

Instead of the main and main1 methods,
it would be better to use unit tests, for example JUnit4 is very easy to use and enabled by default in modern IDEs.

However, to use it, you will also need to rework the detectCycles method to return something instead of printing.
Reading output is not a good way of testing, as it cannot be automated.

Code Snippets

class CycleDetection {
    private final Graph<Node> graph;

    public CycleDetection(Graph<Node> graph) {
        this.graph = graph;
    }

    public void detectCycles(Node source) {
        Deque<Node> stack = new ArrayDeque<Node>();
        Set<Node> visited = new LinkedHashSet<Node>();

        stack.push(source);

        // ...
Deque<Node> stack = new ArrayDeque<Node>();
    Set<Node> visited = new LinkedHashSet<Node>();
Deque<Node> stack = new ArrayDeque<>();
    Set<Node> visited = new LinkedHashSet<>();

Context

StackExchange Code Review Q#109454, answer score: 3

Revisions (0)

No revisions yet.