Path: csiph.com!x330-a1.tempe.blueboxinc.net!usenet.pasdenom.info!aioe.org!news.stack.nl!.POSTED!ipv6.urchin.earth.li!twic
From: Tom Anderson <twic@urchin.earth.li>
Newsgroups: comp.lang.java.programmer
Subject: Re: Automatic linking of related objects in constructor
Date: Thu, 30 Jun 2011 22:51:12 +0100
Organization: Stack Usenet News Service
Lines: 238
Message-ID: <alpine.DEB.2.00.1106302133350.3024@urchin.earth.li>
References: <eYKdnUijTfibapfTnZ2dnUVZ_uadnZ2d@westnet.com.au> <iuf5rd$26m$1@dont-email.me>
NNTP-Posting-Host: ipv6.urchin.earth.li
Mime-Version: 1.0
Content-Type: MULTIPART/MIXED; BOUNDARY="232016332-541013872-1309470672=:3024"
X-Trace: mud.stack.nl 1309470672 89755 2001:ba8:0:1b4::6 (30 Jun 2011 21:51:12 GMT)
X-Complaints-To: abuse@stack.nl
NNTP-Posting-Date: Thu, 30 Jun 2011 21:51:12 +0000 (UTC)
User-Agent: Alpine 2.00 (DEB 1167 2008-08-23)
In-Reply-To: <iuf5rd$26m$1@dont-email.me>
Xref: x330-a1.tempe.blueboxinc.net comp.lang.java.programmer:5798

  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:

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