object != null verification

Question

Is this verification whether the object passed null or not is OK or I should use a kind of equals() methods?

public void addCard(Card card) throws IllegalArgumentException {
        if (card != null){
            cardList.add(card);
        } else {
            throw new IllegalArgumentException();
        }
    }

Answer 1

That's correct, but personally I'd structure it the other way round. It's common to validate arguments at the start of the method, and then use them for the rest of the method knowing they're correct:

public void addCard(Card card) {
    if (card == null) {
        throw new IllegalArgumentException("Attempt to add null card");
    }
    cardList.add(card);
}

The benefit of doing all the argument testing up-front is that if an invalid argument is passed to you, you'll throw an exception before any side-effects have taken place - rather than half way through the method, which could leave the object in an invalid state. Of course in this case it doesn't matter, but I favour consistency here :)

Note that there's no need to declare IllegalArgumentException - it's a subclass of RuntimeException, which means it's unchecked (you don't need to declare it).

Answer 2

I prefer to do it like this:

/**
 * Adds a card to the deck.
 *
 * @param the card to add.
 * @throws IllegalArgumentException if the card is null.
 */
public void addCard(final Card card) 
{
    if(card == null)
    {
        throw new IllegalArgumentException("card must not be null");
    }

    cardList.add(card);
}

1. it is less code to read
2. it separates the error condition from the expected condition (no else = no indent)
3. nobody needs to see what is on line X to see that the card variable was null = less time for people searching to find out what they did wrong.
4. no need to clutter the code with useless throws statements - you should javadoc it though with an @throws clause

Other than that, you are doing it right, in my opinion.

Answer 3

Effective Java from Josh Blosh recommends the following:

/**
 * ads a card to card list.
 * 
 * @param card card to add. Can not be null
 * 
 */
public void addCard(Card card) {
      if (card == null) {
           throw new NullPointerException("Card can not be null")
      }  
      cardList.add(card);
    }

So, firstly you do not declare the RuntimeException. Secondly you throw a NullPointerException because your argument is not simply incorrect - it is null. Thirdly, you specify the arguments in javadoc, can they be null or not.

Answer 4

You can use commons-lang Validate class. It makes your code shorter and simpler:

public void addCard(Card card) {
    Validate.notNull(card);
    cardList.add(card);
}

It will throw an IllegalArgumentException with a default message (you can pass a custom message as a 2nd argument, if you like)