Don't allow getContext to return null..?

[hinerm]

After this SCIFIO ticket I am concerned that there is nothing enforcing Contextual objects from being used, and attempting to use their Context, before their Context is set. This inevitably leads to NPEs with an unclear message.

I think to allow lazy initialization, we should maintain the paradigm of being allowed to construct Contextual objects and set their Context manually later. However, I think the getContext method should throw an exception with an appropriate message if it's called and the Context is null, to fail early in as helpful a way as possible.

We could instead refactor all consuming code to check for null Contexts, but I think the results would be very similar - if you are trying to access the Context of a Contextual object and it is null, there is probably some state that is wrong in the program. Changing at the Contextual level would be safer and simpler and avoid boilerplate and code refactoring.

We could also add a hasContext method to safely check for null Contexts.

@ctrueden @dscho - any thoughts/improvements/objections/etc...?

[ctrueden]

This change makes me nervous, though I'm not sure I can articulate very well why.

While the getContext implementation of AbstractContextual could have code like if (context == null) throw new NullPointerException("Context is not yet set");, I think there are dangers to this. It is probably a legitimate use case to write if (thing.getContext() == null) thing.setContext(myContext); to ensure injection occurs without causing an IllegalStateException with a "Context already injected" message. Essentially, it seems rather nasty to disallow context reinjection and disallow context probing this way (although your proposed hasContext method would mitigate the problem).

In the case of SCIFIO issue #129, the problem would be solved if AbstractGateway did not silently skip the setContext call in the case of a null Context passed to the constructor. Is there some reason for that? Why not let the NPE thrown then, instead of postponing the issue? Would you ever want to have a gateway with a null context, even temporarily?

[hinerm]

Instead of the hasContext method I suggested, @ctrueden and I agreed that just a context method would be superior. This would be functionally the same as getContext with a guarantee of a non-null return value or throwing an exception.

This pattern can then also carry over to the Context itself, where we will add a service method, parallel to getService with a similar non-null/exception guarantee.

[dscho]

a context method would be superior.

:+1:

[ctrueden]

Addressed in f2fb907db46f1af3f099fde169dacf6ad49d5ab3.