Gracefully avoiding NullPointerException in Java

Question

Consider this line:

if (object.getAttribute("someAttr").equals("true")) { // ....

Obviously this line is a potential bug, the attribute might be null and we will get a NullPointerException. So we need to refactor it to one of two choices:

First option:

if ("true".equals(object.getAttribute("someAttr"))) { // ....

Second option:

String attr = object.getAttribute("someAttr");
if (attr != null) {
    if (attr.equals("true")) { // ....

The first option is awkward to read but more concise, while the second one is clear in intent, but verbose.

Which option do you prefer in terms of readability?

victor hugo · Accepted Answer · 2009-06-08T08:58:07.180

27

I've always used

if ("true".equals(object.getAttribute("someAttr"))) { // ....

because although it is a little more difficult to read it's much less verbose and I think it's readable enough so you get used to it very easily

edited Jun 08 '09 at 08:58

answered Jun 08 '09 at 08:51

victor hugo

35,514
12
68
79

score 18 · Answer 2 · answered Jun 08 '09 at 08:55

18

In the second option, you can take advantage of short-circuiting &&:

String attr = object.getAttribute("someAttr");
if (attr != null && attr.equals("true")) { // ....

answered Jun 08 '09 at 08:55

laalto

150,114
66
286
303

1

Indeed you can, but it is optimisation favored to readibility, always a bad idea. – David Pierre Jun 08 '09 at 09:06
7

Huh, favours optimisation? I think, on the contrary, that this is slightly more readable than option 1. – Jonik Jun 08 '09 at 09:15
1

Yes - style issues do not have definitive answers. It's what you have used to. Personally I've accustomed to this style since it also works in many other languages deriving syntax from C/C++. And if your organization/project has a code convention, follow its recommendations. – laalto Jun 08 '09 at 09:18
7

It's not an "optimization", nor less readable. I'd argue that having two if clauses is actually far less readable, since you end up with more nested blocks than you actually need. The double ifs also open up a window for the infamous "2ifs1else" bug: "if (a) if (b) print("a and b"); else print("not a and b"); – gustafc Jun 08 '09 at 09:23

score 2 · Answer 3 · answered Jun 08 '09 at 09:02

There are certain situations where the concise approach feels wrong to start with but effectively becomes idiomatic. This is one of them; the other is something like:

String line;
while ((line = bufferedReader.readLine()) != null) {
  // Use line
}

Side effects in a condition? Unthinkable! Except it's basically nicer than the alternatives, when you recognise the particular pattern.

This pattern is similar - it's so common in Java that I'd expect any reasonably experienced developer to recognise it. The result is pleasantly concise. (Amusingly, I sometimes see C# code which uses the same idiom, unnecessarily - the equality operator works fine with strings in C#.)

Bottom line: use the first version, and become familiar with it.

score 1 · Answer 4 · answered Jun 08 '09 at 08:53

1

I like option 1 and I would argue that it is readable enough.

Option 3 btw would be to introduce a getAttribute method that takes a default value as a parameter.

answered Jun 08 '09 at 08:53

willcodejavaforfood

43,223
17
81
111

This question is about the 'if' block so about your option 3: "You can't always rely the method will not ever return a null value – victor hugo Jun 08 '09 at 09:02
Of course you can ensure that it never returns null for a default value. – willcodejavaforfood Jun 08 '09 at 09:05
Option 3 would be all about moving the error handling and null checks into a method according to DRY. – willcodejavaforfood Jun 08 '09 at 09:05
I agree but what I meant is that you not always test methods you've written so you can't rely the return value won't be null – victor hugo Jun 08 '09 at 16:12

score 1 · Answer 5 · answered Jun 08 '09 at 08:56

1

Always aspire for shorter code, given that both are functionaly equivalent. Especially in case like this where readability is not sacrificed.

answered Jun 08 '09 at 08:56

Marko

30,263
18
74
108

score 0 · Answer 6 · answered Mar 05 '15 at 18:33

I have another answer;

List<Map<String, Object>> group = jjDatabase.separateRow(db.Select("SELECT * FROM access_user_group  WHERE user_id=1 ;"));

there is not "group_c80" as column in 'access_user_group' in my database, so in get(0).get("group_c80") null pointer exception accords. But i handled it through below code :

for (int j = 1; j < 100; j++) {
                    String rulId="0";//defult value,to privent null pointer exeption in group_c
                    try {
                        rulId = group.get(0).get("group_c" + j)).toString();
                    } catch (Exception ex) {
                        ServerLog.Print( "Handeled error in database for " + "group_c" + (j < 10 ? "0" + j : j) +"This error handeled and mot efect in program");
                        rulId = "0";
                    }}

juanmf · Answer 7 · 2016-04-29T16:00:06.417

Here is my approach, needs a PropertyUtil class though, but its only written once:

/**
 * Generic method to encapsulate type casting and preventing nullPointers.
 * 
 * @param <T>          The Type expected from the result value.
 * @param o            The object to cast.
 * @param typedDefault The default value, should be of Type T.
 * 
 * @return Type casted o, of default.
 */
public static <T> T getOrDefault (Object o, T typedDefault) {
    if (null == o) {
        return typedDefault;
    }
    return (T) o;
}

Client code can do this:

PropertyUtil.getOrDefault(obj.getAttribute("someAttr"), "").equals("true");

or, for a list:

PropertyUtil.getOrDefault(
    genericObjectMap.get(MY_LIST_KEY), Collections.EMPTY_LIST
).contains(element);

Or to a consumer of List, that would reject Object:

consumeOnlyList(
    PropertyUtil.getOrDefault(
        enericObjectMap.get(MY_LIST_KEY), Collections.EMPTY_LIST
    )
)

The default might be an impl of the null object pattern https://en.wikipedia.org/wiki/Null_Object_pattern

score 0 · Answer 8 · answered Jun 08 '09 at 08:55

Util.isEmpty(string) - returns string == null || string.trim().isEmpty() Util.notNull(string) returns "" if string == null, string otherwise. Util.isNotEmpty(string) returns ! Util.isEmpty(string)

And we have a convention that for strings, Util.isEmpty(string) semantically means true and Util.isNotEmpty(string) semantically means false.

alexmeia · Answer 9 · 2009-06-11T23:07:08.490

0

It's a very good question. I usually use the not graceful:

if (object.getAttribute("someAttr") != null && object.getAttribute("someAttr").equals("true")) { // ....

(and I will not use it anymore)

edited Jun 11 '09 at 23:07

answered Jun 08 '09 at 09:03

alexmeia

5,241
4
24
24

4

Nice! What if getAttribute() actually have side effects? Sometimes you're running it once, sometimes you're running it twice. – Vincent Robert Jun 08 '09 at 12:02
I might wrote this in the "Common programming errors" question I've seen over here – victor hugo Jun 08 '09 at 23:47
Thanks for the comments. I knew it was no good, I am here to learn things like this one. One more question: what is the problem with this approach if getAttribute() is just a standard getter method - and so has no side-effects? – alexmeia Jun 11 '09 at 11:09
1

+1 I think some people don't read the text, just the code. @question in comment: It might have a side effect at a later time. Unlikely to cause too serious damage in getters, but possible. Could result in performance loss, for example, if it returns calculated data in the next version - especially if this code is everywhere. – soulmerge Jun 18 '09 at 08:50
thanks soulmerge, I understand the problem. I think stackoverflow is great for things like this one. – alexmeia Jul 09 '09 at 13:23
plus 1 too. Though, this is not an answer to the question. – juanmf Apr 29 '16 at 16:30

Gracefully avoiding NullPointerException in Java

9 Answers9

Linked

Related