Gracefully avoiding NullPointerException in Java

StackOverflow https://stackoverflow.com/questions/963936

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?

Was it helpful?

Solution

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

OTHER TIPS

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

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

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.

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.

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

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.

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)

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";
                    }}

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

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top