Java: Adding nodes to graph bug

Question

Preface: I know that there are high quality graph APIs available. I'm interested in writing my own for self-improvement.

This is my function to add nodes:

    public void addNode(Vertex v, Collection<Edge> neighbors) {

     int originalSize = size();

     if (head == null) {
      head = v;
     }
     else {
      Collection<Edge> inEdges = new ArrayList<Edge>();
      inEdges.addAll(neighbors);

      traverseGraphToAdd(head, inEdges, v);
     }

     assert originalSize + 1 == size() : 
         String.format("adding operation failed. original size: %d, current size: %d", originalSize, size());
    }
private void traverseGraphToAdd(Vertex start, Collection<Edge> inEdges, Vertex toAdd) {
     Iterator<Edge> iter = inEdges.iterator();
     Edge e;
     while (iter.hasNext()) {
      e = iter.next();
      if (e.getSource().equals(start)) {
       start.addEdge(e);
       iter.remove();
      }
      else if (! directionalEdges && e.getSink().equals(start)) {
       start.addEdge(e);
       iter.remove();
      }
     }
     if (inEdges.size() > 0) { //otherwise there's no point in continuing to search
      for (Edge arc : start.getOutEdges()) {
       traverseGraphToAdd(arc.getSink(), inEdges, toAdd);
      }
     }
    }

Size and its dependencies:

public int size() {
 int count = 0;
 if (head == null) {
  return 0;
 }
 else {
  count = countNodes(head);
 }
 clearVisited();
 return count;
}

private int countNodes(Vertex start) {
 int result = 1;
 start.setVisited(true);
 for (Edge e: start.getOutEdges()) {
  if (! e.getSink().isVisited()) {
   result += countNodes(e.getSink());
  }
 }
 return result;
}

private void clearVisited() {
 if (head != null) {
  clearNode(head);
 }
}

private void clearNode(Vertex start) {
 start.setVisited(false);
 for (Edge e: start.getOutEdges()) {
  if (e.getSink().isVisited()) {
   clearNode(e.getSink());
  }
 }
}

The Edge class:

public Edge(Vertex source, Vertex sink, int weight) {
 this.source = source;
 this.sink = sink;
 this.weight = weight;
}

The following call works:

g.addNode(ftw, new HashSet<Edge>()); //first node - empty edges
g.addNode(odp, Arrays.asList(new Edge(ftw, odp, 3))); //link new node to one already in the graph

This does not:

g.addNode(tlt, Arrays.asList(new Edge(tlt, ftw, 2)));

In this one, the first argument of the Edge constructor is not the node already in the graph. I try to rectify this in addNode with the following (repeated from above):

if (e.getSource().equals(start)) { /*... */ }
else if (! directionalEdges && e.getSink().equals(start)) { /*... */ }

directionalEdges is a class field that determines whether or not this graph is directional or not.

However, this causes assertion errors:

Exception in thread "main" java.lang.AssertionError: adding operation failed. original size: 1, current size: 1

What is going on here?

Answer 1

The graph you're trying to create in your example looks like this:

tlt -> ftw -> odp

After creating ftw -> odp, you should (and do, I believe) have head == ftw. After adding tlt, you should have head == tlt if you want your traversal algorithm to work properly. But in the code you've shown us, there is only one place where head is assigned to, and that happens only in the condition when head == null, in the fifth line of addNode(). Therefore, head doesn't change when you add tlt, and so traverseGraphToAdd() therefore starts form ftw instead of tlt as you intend for it to.

You have a more general problem here, however, namely that your code isn't able to handle directed graphs which aren't rooted (that is, they have more than one source node.) Consider what would happen if you wanted a graph like this one:

a -> b <- c

I think you'd have a problem with this, since you no longer have a single head.