Re: Automatic linking of related objects in constructor

  This message is in MIME format. The first part should be readable text,
  while the remaining parts are likely unreadable without MIME-aware tools.

--232016332-541013872-1309470672=:3024
Content-Type: TEXT/PLAIN; format=flowed; charset=UTF-8
Content-Transfer-Encoding: 8BIT

On Wed, 29 Jun 2011, Eric Sosman wrote:


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.

+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:


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
--232016332-541013872-1309470672=:3024--