On 1/30/14 1:09 AM, Daniel Fuchs wrote:
I wonder whether you should replace the assert in the
constructor by an explicit null check:

-  assert systemStub != null
+ if (systemStub == null) throw new NullPointerException();

The reason I see is that before your change, an object constructed
with a null systemStub would have sooner or later failed in NPE.
Now with your change, an object constructed with a null system
stub will block - waiting forever for system stub to become not
null.

The question of course is whether throwing NPE in  the constructor
would cause any compatibility issues. Passing the JCK might help
to figure it out.

I considered an explicit null test, or something like Objects.requireNonNull, but decided against them. The reason is that this is a private nested class, and there is only one caller elsewhere in the containing class. We just need to make sure this is right, and there's no use checking at runtime.

I was going to add a comment saying "systemStub must be non-null" but then I realized Hey! That's what assert statements are for.

Rmid creation always goes through this path, and this is done repeatedly by the tests. Tests are run with assertions enabled so I think we're well covered here.

s'marks

Reply via email to