Java: Is it ok to set Integer = null ?

Question

I have a function that returns an id number if the argument exists in the database. If not, it returns null. Is this begging for a null pointer exception? Negative id numbers are not permitted, but I thought it would be clearer to have non-existent arguments returning null instead of an error code like -1. What do you think?

private Integer tidOfTerm(String name) throws SQLException {
    String sql = "SELECT tid FROM term_data WHERE name = ?";
    PreparedStatement prep = conn.prepareStatement(sql);
    prep.setString(1, name);
    ResultSet result = prep.getResultSet();

    if (result.next()) {
        return result.getInt("tid");
    }

    return null; // TODO: is this begging for a null pointer exception?
}

Answer 1

No, it won't. It will only throw a NPE if you do operations on it afterwards as if it is a primitive without nullchecking it. E.g. i++ and so on. Your example is valid (expect from that the JDBC code itself is leaking resources). If you don't need the actual id, then you can on the other hand also just return a boolean.

Answer 2

I think it is legitimate to return null in this case. Just make sure that the intention is documented properly.

Returning a negative value in this case would be ok, but it is not an all-around solution. What if negative values were allowed in the db?

EDIT: I want to add a word about the related discussions on SO regarding returning nulls or empty lists (or arrays). I am in favor of returning empty lists or arrays instead of null, but the context is different. When trying to obtain a list, it is typically part of a parent object, and it actually makes sense for the parent object to have an empty list instead of a null reference. In this case, null has a meaning (= not found) and there is no reason to avoid returning it.

Answer 3

I hope this isn't a real method for you. You aren't closing Statement or ResultSet in method scope.

Answer 4

I agree with the posters. Integer is a wrapper and as such should be used for calculations, conversions, etc (which I think you're intending to do). Don't return a null, use a negative number... it's a bit more elegant and allows you for more control. IMHO.

Answer 5

Yes it should be causing an NPE and Yes you should be catching that in the calling method (or some other suitable place). The most likely reason why your method will return NULL is when there are no records and the proper way to handle that is by throwing an exception. And the perfect exception to tell someone that you don't have what he asked for is NPE.

Returning an error code (eg -1) is no good because:

a) if there are many errors you want to handle (eg cannot read DB, can read DB but object does not exist in DB, found object but something is corrupted, etc), then returning an error code does not distinguish between the types of errors.

b) in the future if -1 becomes a legal term id, then it will be hard to change it (if you must use -1, then (EDIT: in C) at least do #define ERRORCODE -1 and use ERRORCODE everywhere)

Answer 6

This is perfectly legal. If you want to avoid a NPE, throw a custom exception. But don't return a negative number. If the caller doesn't check the return value, you will always have a problem. But doing false calculation (because the result is for instance multiplied by -1) is definitely harder to debug than an uncatched exception.

Answer 7

It might cause big trouble for novice users. Good coders will recognize that if name is invalid then null might be returned. Having said that the more standard thing is to throw some exception.

Answer 8

Returning a null in the case of a lookup that does not give a result is a normal method of representing non-existance. I would choose it in this case. (Lookup methods for the standard Java Map classes are an example for use of null in case the map does not contain a key.)

As for returning a special value for the ID, I would only propose to do that if your system already contains special values repesenting special ID's.

Another often heard possibility is throwing an exception in this case. It is not wise however to use exceptions to pass state, so I would not do that either.

Answer 9

Might be tricky in combination of Autoboxing. If i am doing this:

final int tid = tidForTerm("term");

And "term" does not exist, i will get a NPE, because Java tries to unbox an Integer (null) to a primitive int.

Nevertheless, there are cases where it's actually good to use null for Integers. In entities having optional int values, e.g. the population of a city. In that case null would mean, no information available.

Answer 10

Please do not write code that returns null. This means that each and every call to your code MUST check for null in order to be robust. Everytime. Always.

Consider returning a list instead containing number of return values, which might be zero.

Answer 11

• Do not use an error code! Which value is the error? will it never become a legal return value? Nothing won.

• Null is not good. Most caller code have to do a if not null check for the result. And some times the select may return null. Should it be handled different than no row?

• Throw an exception like NoSuchElementException instead of the return null. It is an unchecked exception, the caller can handle it or pass it up. And if the caller wants to handle, the try catch is not more complex than an if not null.

Answer 12

An interesting problem and a large number of possible solutions:

1. Create an HasArgument method and require users to call it, problem this may be slow and dupplicate work
2. Throw an exception if a value is not in the database, should only be used if this is unexpected
3. Use additional values to indicate an invalid return, null and negative values would work for you but if not checked they could cause problems later in the code.
4. Return a wrapper with an isValid() and a getValue() method, where getValue() throws an exception when it is invalid. This would solve the problems of 1 and 3, but may be a bit too mutch.

Answer 13

I suggest you consider the option pattern.

The option pattern acts as a wrapper around your returned type, and defines two special cases: option.none() and option.some(). That way, you always know your returned type (an option), and can check if you have a value in your returned Option object using methods such as option.isSome() and option.isNone().

This way, you can guarantee you don't have any unchecked nulls.

Of course, all this comes at the cost of additional code complexity.

For more information on the option type, see here (Scala code, but same principal)

Answer 14

I'd say that the best solution depends on what your method is named, and you should consider what your methods are named as much as whether they should return null or throw an exception.

tidOfTerm implies to me that the Term is expected to exist, so discovering that one doesn't exist should throw an exception.

If the term name is under the control of your own code, and not finding it indicates a bug in your code or environment, then you might want to throw an IllegalArgumentException.

If the term name argument is not under your control, and not finding a valid term is a perfectly valid situation, then I'd rename your method to something like findTidForTermName giving a slight hint that some kind of search is going to be performed, and that there is therefore the possibility that the search might not find anything.