2

I'm trying to get two nodes to link together. When n adds s as a link, s should also update to add n as a link. But the code calls itself and gets stuck in an infinite loop, then overflows. How can I get the nodes to assign to each other, but not recursively assign themselves?

public class Node {
    Set<Node> connections = new HashSet<Node>();

    public static void main(String args[]) {
        Node n = new Node();
        Node s = new Node();
        n.addNode(s);
    }

    public Node() {

    }
    public void addNode(Node newNode) {
        connections.add(newNode);
        newNode.addNode(this);
    }
}

Update: I added this code to have the method call another setter method.

   public void addNode(Node newNode) {
        connections.add(newNode);
        newNode.addSingleNode(this);
    }
    protected void addSingleNode(Node newNode) {
        connections.add(newNode);
    }
}
Mureinik
  • 297,002
  • 52
  • 306
  • 350
Alex G
  • 747
  • 4
  • 15
  • 27

2 Answers2

4

Don't use recursion for this, and you won't have a problem.

public void addNode(Node newNode) {
    connections.add(newNode);
    newNode.connections.add(this);
}

This is allowed even if connections is private, because access to fields and methods is controlled on a per-class basis, not a per-object basis.

Dawood ibn Kareem
  • 77,785
  • 15
  • 98
  • 110
  • So, I realized this and thought about doing it, but I was worried because that means `connections` is being directly modified, without error checking. Would it be better to use a protected method to set it? I'll post an edit in a second. – Alex G Oct 12 '17 at 20:35
  • 1
    You can add whatever error checking you like to this method. – Dawood ibn Kareem Oct 12 '17 at 20:37
  • 1
    Interesting. Wasn't aware of the `private` designation still allowing this to work. To be clear, you're saying that object A and object B can both modify each other's private variables, if they're the same type? Cool. – Alex G Oct 12 '17 at 20:39
  • Yes, this is correct. `private` only hides the variable to **other classes**, not other instances of the same class (see [SO: Access private field of another object in same class](https://stackoverflow.com/questions/17027139/access-private-field-of-another-object-in-same-class) for an explanation and further links). – Zabuzard Oct 12 '17 at 20:53
  • The way I think about this is that **code** written in class `X` has access to `private` members declared in class `X`. If `Y` is a subclass of `X`, then an object of class `Y` can have code from both classes; but it's _where the code is written_ that dictates which private members it can see. And yes, if a private member is declared in class `X`, then the code in class `X` can always access it, even it the member belongs to a different object from the one referenced by `this`. – Dawood ibn Kareem Oct 12 '17 at 21:08
2

You could just access the other's node member directly:

public void addNode(Node newNode) {
    connections.add(newNode);
    newNode.connections.add(this);
}

An arguably "cleaner" way of doing this is to encapsulate this "logic" in a method:

private void addConnection(Node newConnection) {
    connections.add(newConnection);
}

public void addNode(Node newNode) {
    addConnection(newNode);
    newNode.addConnection(this);
}
Mureinik
  • 297,002
  • 52
  • 306
  • 350
  • Any downside to the direct way, or any reason to use the "cleaner" way? – Alex G Oct 12 '17 at 20:38
  • 2
    The downside is **cluttering**, more methods can clutter your code. However there are many pros: You now have a place to easily insert logging, error-checking or future modifications. You can even totally exchange the internal data structures but the call to the method will remain the same. – Zabuzard Oct 12 '17 at 20:40
  • 1
    @AlexG With `addConnection` being private, it's mostly a matter of style. If you make it protected, there's an argument to be made for it allowing you to extend the `Node` class and allow different extending classes to implement it differently. – Mureinik Oct 12 '17 at 20:41
  • Okay. To be clear, this is going to be extended by another class (it's for mapping Rooms in a game level). But they won't be modifying the code at all, so it can stay private I think. Btw I believe your "direct" method is off a little. Should be `newNode.connections.add(this)`. – Alex G Oct 12 '17 at 20:44