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?
}

Catechize

You return statement could be shorter: return result.next() ? result.getInt("tid") ? null; – Lowspirited

@Willi - Shorter != more readable – Cloots

Agreed in general, but it would read much better in the example above. – Lowspirited

Answer 1

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.

Marnamarne

Great point, there is indeed danger that the caller will perform a false calculation. That's why I think documentation is so important. As I mentioned in my answer, if I already document the return type, I might as well make it null – Killick

In Java 8 (still quite a few years away when the question was asked) you should use Optional<Integer> for this purpose. Adding this comment here on the accepted answer in case someone stumbles across this. – Saturate

Actually, OptionalInt. – Finally

Answer 2

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.

Volk

Regarding your last sentence: Why is it not wise to use exceptions to pass state, and what state would be passed in this instance? – Fulltime

In this instance the passed state would be the non-existence of the name in the table. Exceptions are meant to represent exceptional situations that do not occur normally, while state is passed around during normal execution - use method arguments and return values for passing state. (And the stack unwinding by thrown exceptions is less performant than a return value.) – Volk

Answer 3

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.

Killick

Answer 4

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

Leafy

Good points, although this isn't really an answer to the question. – Brede

Correct, but there are times when sticking to the question isn't the best policy. Others have given a perfectly acceptable answer that meets the letter of the law. Why simply repeat it? – Leafy

I think @TM's point was to allude you should have posted a comment and not an answer. – Porous

@Porous - yes, I know what the point was. I don't happen to agree with it. What I said is entirely correct and appropriate, and whether I put it in an answer or comment seems irrelevant to me. Why should it matter? – Leafy

@Leafy - because if you put in an answer when it doesn't have anything to do with the question its clear your just trolling for rep. meta.stackexchange.com/questions/17447/… – Porous

Still just an opinion, not fact. Are you saying that people need not be made aware that failing to close resources is a bad thing? I think it is a good answer. And no, I'm not reputation whoring or trolling or any other negative thing that you feel entitled to say. Next time Jon Skeet gives an answer that's a millimeter off topic I'll send him over to you so you can call him names, too. – Leafy

@harschware, maybe you should get a down vote because you don't know the difference between "it's" and "its". Were you nominated to be a monitor or something? How are these comments of yours pertinent to answering anything? – Leafy

@duffymo: people should be made aware, it was an excellent point that would be fine for a comment. I didn't downvote you, cause it was a minor mistake. OK, "Trolling" was a bit negative... happy SO'ng. – Porous

Answer 5

• 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.

Key

Answer 6

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)

Ectoparasite

Answer 7

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.

Childish

Answer 8

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.

Syntactics

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.

Lowspirited

Answer 10

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.

Edra

Answer 11

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.

Recant

Answer 12

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.

Chung

one risks doing calculations, conversions, etc with the negative number if one forgets to check for it. If one does check for negative numbers, one could as well check for null. – Alagez

Answer 13

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)

Mischievous

Did you actually just suggest using #define in a Java question? (I almost -1) – Porous

Not telling you to throw an NPE. But you should happily return null and handle that in the calling function. If you access it in the calling function when it is null, you should catch the NPE and work from there. Think I didn't make it clear in the original post, but you get my drift. – Mischievous

Answer 14

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.

Unemployment

How is "return a List" relevant when the return type is Integer? – Leafy

Returning null is perfectly valid, and very common amongst countless APIs. As long as you document it, it's absolutely the right thing to do. – Rowdy

@Skaffman. Countless NPE's have occurred from programmers not checking the return value against null from such API's. Additionally when you have an `a().b().c()' construction in a line you cannot tell immediately which one of the calls gave the NullPointerException. I think Jetbrains are absolutely right in their @NotNull annotation. jetbrains.com/idea/documentation/howto.html – Coastguardsman

@duffymo, in this particular case you cannot and a change would be necessary. Do you otherwise agree or disagree with the attitude towards null return values? – Coastguardsman

Looks like in this particular case that the method is returning an id, a primary key. That's how Hibernate marks an instance as being unsaved: onjava.com/pub/a/onjava/2006/09/13/…. Is Hibernate dead wrong? I don't think so. – Leafy

I agree that the guideline should be not to return null in most cases - but "know the rules, and know when to break the rules" is still appropriate. – Leafy

Effective Java agrees that you should return an empty object rather than null in order to save the caller a ==null check – Venial