What’s ‘wrong’ with this code ? #01

I came accross a piece of code like this:

	if (null == sessionProxy.getShoppingCart()) {
		throw new IllegalStateException("ShoppingCart may not be null");
	}

Can you find what’s ‘wrong’ with this? Note I have put wrong in quotes.

Comments

  1. I see a couple of things wrong with this design. If the shopping cart should not be null then the sessionProxy should throw the exception. This could be at creation time of the proxy or at retrieval time. Otherwise this null-check will be scattered all throughout your codebase.

    Also the type of Exception thrown could be a lot clearer.

    I do like the Yoda-condition, though 😉

    1. With yoda you mean the piece “null == something” ?

      I’ll post my answer at the end of the day.

      1. Exactly. In the good old days Yoda conditions were used as a trick to catch typo’s in the equality-check. “if (someVariable = null)” is valid but wrong. However “if (null = someVariable)” will throw a compile-time error.

        1. you mean assignments in an if statement should throw a compile time error? In this case it is just checking for null. Although if (null == somevar) , should do the same as (somevar == null). I believe this is taken from the string equals checks, which can be null. Ie:

          if (somevar.equals(“BLA”))

          can throw NPE when somevar is null, while:

          if (“BLA”.equals(somevar)) will not throw NPE when somevar is null.

      2. If you type “=” instead of “==” by accident, Yoda conditions will have your back 😉 For a more elaborate explanation, check out: http://wiert.me/2010/05/25/yoda-conditions-from-stackoverflow-new-programming-jargon-you-coined/

        Nowadays you get a compiler error or warning in case of these errors, so you notice these errors faster now. More importantly, you should have a failing test for this.

        We’re going way off-topic here, btw 😉

Leave a Reply to stefanhendriks Cancel reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.