“Leaking this” from a Design Standpoint

Question

Warning: Leaking "this" in constructor

I keep running into this, and I have a nagging feeling that it's because my design is wrong or not optimal.

I understand that this warning is bringing to my attention the fact that I am allowing access to an object that is potentially not fully initialized.

Let's say that I need a Frame that HAS and requires a List (Frame(List list)). In List, I might want to do something such as add(). In order to make sure Frame knows as little about List as possible (only that it has one), I would want to access the containing Frame from the List (List HAS a Frame?). This seems a little silly, but I have 2+ implementations of List that will use Frame in different ways..

To ensure that my code is used properly, I would require a Frame in the constructor of List. I would also require a List in the constructor of Frame, as it MUST have one:

public abstract class Frame {
    private final List list;

    public Frame(List list) {
        this.list = list;
        list.setFrame(this);
    }
}

public abstract class List {
    private Frame frame;

    protected final void setFrame(Frame frame) {
        this.frame = frame;
    }
}

So, is this bad design, or should I really create some intermediate scaffolding that does this, or even leave the scaffolding to the user?

Thanks!

Answer 1

I think that this sort of "doubly linked" structure, where a Frame points to a List that points back to its parent Frame, is something you ought to avoid unless you have a specific need for it. You should if possible try and make one of the two objects the "parent" that points to the "child."

It's a bit hard to understand at first why the double-linking is not such a great idea, but here are some reasons:

1. In a structure with clear parent-child relationships, it's trivial to reuse the same child objects in more than one parent. In a double-linked structure, on the other hand, if you want to create a new structure that shares some of the elements from an original, you either have to create new copies of the elements of interest, or you have to destroy the original structure.
2. There are many design patterns that rely on a clear chain of "command" in an object graph, and the double-linked structures obscure that chain of command from readers of the code.
3. A lot of the cases where processing of a child requires knowledge about the parent in which that child appears are better handled by passing the parent as a "context" argument to a method that processes the child. The classic example of this pattern is in interpreters, where evaluation of an expression takes as an argument an "environment" that contains all the of the outside information necessary to evaluate a child expression. (Or alternatively, you use a stateful Iterator or Hierarchical Visitor that navigates the structure and keeps track of location.)

This is not to say that the double-linked structures are never appropriate, but rather that the simpler, single-linked structures should probably be the first choice.

Answer 2

Introduce a factory method:

public static Frame createFrame(List list) {
    Frame frame = new Frame(list);
    list.setFrame(frame);
}

private Frame(List list) {
    this.list = list;
}

This does not leak this, and always makes sure everything is configured correctly without the need for every caller to remember initializing both sides of the association.