Re: Automatic linking of related objects in constructor

Discussion in 'Java' started by Eric Sosman, Jun 29, 2011.

  1. Eric Sosman

    Eric Sosman Guest

    On 6/29/2011 5:56 AM, Qu0ll wrote:
    > Suppose you have class A which contains a list of objects of class B and
    > that the constructor for B takes in a reference to the A object which is
    > to include it in said list. Is it safe to make a call to A's method
    > addB(B b) in that B constructor passing in "this"? I have heard that
    > it's bad practice to "leak" a reference to an object from within its own
    > constructor because it may be in an invalid state.


    I guess you mean

    class B {
    public B(A a) {
    ...
    a.addB(this);
    }
    ...
    }

    It's not a wonderful idea, but even if it's not safe it looks sort
    of safe-ish, right? Ah, but code changes, and lo! one of the
    changes introduces a subclass:

    class C extends B {
    public C(A a) {
    super(a);
    ...
    }
    }

    Okay, still not *too* bad -- or is it? Let's expand that last
    set of ellipses:

    class C extends B {
    public C(A a) {
    super(a);
    if (phaseOfTheMoon() != Moon.FULL)
    throw new IllegalStateException();
    ...
    }
    }

    Yikes! Now if C's constructor throws up, the A instance is left
    holding a reference to something that appears to be a C in B's
    clothing, but is in fact nothing sensible at all!

    > If not, how else can I automatically add the B object to the list in A
    > without forcing the client programmer to explicitly call addB() given
    > that they have already passed in the B as an argument?


    (I assume you meant "A as an argument.") Consider using a
    factory method:

    class B {
    private B(A a) {
    // so clients can't do "new B(a)"
    ...
    }
    public static B instanceOf(A a) {
    B b = new B(a);
    // B's constructor is now completely finished
    a.addB(b);
    return b;
    }
    }

    --
    Eric Sosman
    d
    Eric Sosman, Jun 29, 2011
    #1
    1. Advertising

  2. Eric Sosman

    Tom Anderson Guest

    On Wed, 29 Jun 2011, Eric Sosman wrote:

    > On 6/29/2011 5:56 AM, Qu0ll wrote:
    >
    >> Suppose you have class A which contains a list of objects of class B
    >> and that the constructor for B takes in a reference to the A object
    >> which is to include it in said list. Is it safe to make a call to A's
    >> method addB(B b) in that B constructor passing in "this"? I have heard
    >> that it's bad practice to "leak" a reference to an object from within
    >> its own constructor because it may be in an invalid state.


    It can be safe, but it can also be unsafe, as Eric explained in the
    passage i have so rudely snipped. It is widely considered that if
    something can be unsafe, then it is unsafe.

    That said, if you control A and B, and you are confident that nobody who
    might work on A or B, or add subclasses or whatever, will do something
    wrong, then it is safe. But that's a gamble. Or a tradeoff, as some people
    call it.

    >> If not, how else can I automatically add the B object to the list in A
    >> without forcing the client programmer to explicitly call addB() given
    >> that they have already passed in the B as an argument?

    >
    > (I assume you meant "A as an argument.") Consider using a
    > factory method:
    >
    > class B {
    > private B(A a) {
    > // so clients can't do "new B(a)"
    > ...
    > }
    > public static B instanceOf(A a) {
    > B b = new B(a);
    > // B's constructor is now completely finished
    > a.addB(b);
    > return b;
    > }
    > }


    +1.

    A slightly qualified +1.

    Note that this prevents subclassing of B - the constructor is private. If
    someone later wants to add a subclass, they will have to revisit this, and
    will hopefully change it in a way which preserves the
    b.getA().getBs().contains(b) invariant for the subclass. However, that's
    another gamble - they might not. And once the constructor is protected
    rather than private, the door is open to further subclasses which do not
    preserve the invariant. This situation might actually be *less* safe than
    handling the addition in the B constructor; you trade risk of exposing
    incompletely constructed instances of B for risk of creating
    inconsistently configured instances of A.

    As supercalifragi... you know, i'm just going to call him Supes, said:

    > But if B objects are supposed to all be tracked by an A, that suggests
    > that B objects' lifecycles should be managed by A objects rather than
    > directly by end-users


    I don't really buy into any of his solutions, so i'll add another -
    parental advisory, i haven't compiled any of this, so it might be wrong in
    the details. We already know that:

    class A {
    private final Set<B> bs;
    Set<B> getBs() {
    return Collections.unmodifiableSet(bs);
    }
    }

    class B {
    private final A a;
    A getA() {
    return a;
    }
    }

    And we would like to ensure that:

    /* ∀ */ B b; assert b.getA().getBs().contains(b);

    So i would suggest that:

    class A /* again */ {
    class Provider {
    A getA() {
    return A.this;
    }
    private Provider() {}
    }
    }

    class B /* again */ {
    static interface Constructor<T extends B> {
    T newInstance(A.Provider aProvider);
    }
    static final Constructor<B> CONSTRUCTOR = new Constructor<B>() {
    B newInstance(A.Provider aProvider) {
    return new B(aProvider);
    }
    }
    protected B(A.Provider aProvider) {
    this.a = aProvider.getA();
    }
    }

    class A /* for the last time */ {
    <T extends B> T addB(B.Constructor<T> bCtor) {
    T b = bCtor.newInstance(new Provider(this));
    bs.add(b);
    return b;
    }
    }

    Which is then easily used by client code:

    A a;
    B b = a.addB(B.CONSTRUCTOR);

    I would shoot on sight anyone who actually did this, but i think what i've
    done here is made it impossible to construct an instance of B or a
    subclass of B for which the invariant does not hold.

    To illustrate this, if you want to write C, a subclass of B, you have to
    do:

    class C extends B {
    public C(A.Provider aProvider) {
    super(aProvider);
    }
    }

    You are forced to do this because B constructor requires an A.Provider,
    and since you can't create them, the only way to get one is as a
    parameter. But since the only way to get one as a parameter is through
    B.Constructor.newInstance, you have no choice but to provide some
    plumbing:

    class C extends B /* again */ {
    static final Constructor<C> CONSTRUCTOR = new Constructor<C>() {
    C newInstance(A.Provider aProvider) {
    return new C(aProvider);
    }
    }
    }

    You could do that differently (with a named class or whatever), but if you
    want to create instances of C, it has to boil down to something like that.

    Usage is again:

    A a;
    C c = a.addB(C.CONSTRUCTOR);

    Note that you could get round the static checking by passing a null to the
    B constructor, but that will immediately blow up at runtime, when B's
    constructor calls aProvider.getA(), so it doesn't actually let you create
    anything incorrect.

    Anyway, having written that Constructor implementation, the only code
    which can call it is A.addB, because only that can create instances of
    A.Provider. So it's all still safe.

    The key moves are (i) the constructor of B won't let anyone create a B, or
    a subclass of B, unless they can provide an instance of ProviderA, and
    (ii) only A has the power to create instances of ProviderA. So, only
    instances of A can create instances of B or subclasses of B, and when they
    do so, they also add them to their set. The rest of it is just
    set-dressing, MacGuffins, and matchmaking. A.Provider in particular is a
    class that does absolutely nothing, but which serves as an unforgeable
    token.

    Now, A.Provider is unforgeable, but as written, they are reusable. A
    villain could write:

    class X extends B {
    static final Constructor<X> CONSTRUCTOR = new Constructor<X>() {
    X newInstance(A.Provider aProvider) {
    return new X(aProvider);
    }
    }
    public A.Provider muaHaHa;
    X(A.Provider aProvider) {
    super(aProvider);
    this.muaHaHa = aProvider;
    }
    }

    class Z extends B {
    Z(X coConspirator) {
    super(coConspirator.muaHaHa);
    }
    }

    A a;
    X x = a.addB(X.CONSTRUCTOR);
    while (true) {
    Z z = new Z(x);
    assert !b.getA().getBs().contains(b); // YOUR HEAD A SPLODE
    }

    However, for the truly paranoid, this can be dealt with:

    class A /* special guest appearance */ {
    class Provider /* again */ {
    private boolean used = false;
    A getA() {
    if (used) throw new IllegalStateException();
    used = true;
    return A.this;
    }
    private Provider() {}
    }
    }

    I think that's now watertight. Bar cheating.

    As i said, this is not actually a good idea - too clever for its own good,
    and probably has some gaping vulnerability i haven't thought of - but it
    does go to show what you can do with the combination of strong types and
    access modifiers. You just try that in Ruby!

    tom

    --
    The purpose of local government was to demonstrate, on a small scale,
    what identical bastards the LibDems inevitably became if they ever came
    close to power. -- Quercus
    Tom Anderson, Jun 30, 2011
    #2
    1. Advertising

  3. On 30/06/2011 5:51 PM, Tom Anderson wrote:
    > As i said, this is not actually a good idea - too clever for its own
    > good, and probably has some gaping vulnerability i haven't thought of -


    1. setAccessible(true) followed by your choice of reflection dirty
    tricks -- either on A, or B, or even the "unmodifiable" Set
    returned by A.getBs().

    2. public class C extends B implements Cloneable, Serializable

    followed by clone, round-trip through ObjectFooStreams and a byte
    array or disk file, etc. etc.

    3. Native code hacks -- pass a B to a native method that then goes to
    town on it with C pointer arithmetic and unsafe casts.

    4. Assorted byte code hacking.

    Of course 1 won't work in e.g. unsigned applets, nor 3, and 4 probably
    won't pass the bytecode verifier in stock JVMs, though 4 combined with
    gcj or Jet compilation to native code might work. 2 is the biggest hole
    but you can implement clone and writeObject in B to throw exceptions to
    plug it. Note that just copying the object by either method will break
    the invariant, and serialization adds the ability to further hack the
    serialized object while it's in the form of a defenseless byte array or
    disk file.

    If you want safety combined with serialization you need the B-has-a-C
    strategy pattern approach, I suspect.
    supercalifragilisticexpialadiamaticonormalizeringe, Jun 30, 2011
    #3
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. Guest
    Replies:
    1
    Views:
    749
    Guest
    Jun 29, 2004
  2. Replies:
    1
    Views:
    312
    Jonathan Mcdougall
    May 23, 2006
  3. Replies:
    1
    Views:
    301
    Victor Bazarov
    May 23, 2006
  4. Generic Usenet Account
    Replies:
    10
    Views:
    2,202
  5. Yuri Leikind
    Replies:
    0
    Views:
    107
    Yuri Leikind
    Sep 2, 2003
Loading...

Share This Page