3

I had this warning in Android-studio that told me:

Method invocation 'data.getExtras().get("address").toString()' may produce 'java.lang.NullPointerException'

So I changed my code to get rid of that warning.

// Function to read the result from newly created activity @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { super.onActivityResult(requestCode, resultCode, data); if (resultCode == 100 && data.getExtras().get("x") != null && data.getExtras().get("y") != null && data.getExtras().get("address") != null) { String sX = data.getExtras().get("x").toString(); String sY = data.getExtras().get("y").toString(); String sAddress = data.getExtras().get("address").toString(); double dX = Double.parseDouble(sX); double dY = Double.parseDouble(sY); ShowSearch(dX, dY, sAddress); } else{ Log.d("onActivityResult()", "Something went wrong, either the result code is wrong or the data is null"); } } 

Then on second thought, I opted for a try catch.

// Function to read the result from newly created activity @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { super.onActivityResult(requestCode, resultCode, data); if (resultCode == 100) { try { String sX = data.getExtras().get("x").toString(); String sY = data.getExtras().get("y").toString(); String sAddress = data.getExtras().get("address").toString(); double dX = Double.parseDouble(sX); double dY = Double.parseDouble(sY); ShowSearch(dX, dY, sAddress); } catch (java.lang.NullPointerException e){ Log.d("onActivityResult()", "Something went wrong, some data is null"); } } } 

But using a try catch brings back the warning in android-studio when I'm pretty sure it shouldn't because whether it's null or not, I'm handling it now.

Here's my question, which of the two solution is technically more efficient, if it's the try catch solution why does Android Studio keeps giving me a warning?

(Android Studio 2.1.1)

UPDATE: After trying multiple solutions, I realized that android studio gives me a warning even on the first example so I still have that warning but it's not bothering me anymore.

To those interested here's the NEW solution I decided to use: (I still get warnings)

// Function to read the result from newly created activity @Override protected void onActivityResult(int requestCode, int resultCode, Intent data) { super.onActivityResult(requestCode, resultCode, data); if (resultCode == 100 && data != null) { if (data.hasExtra("x") && data.hasExtra("y") && data.hasExtra("address")) { if (data.getExtras().get("x") != null && data.getExtras().get("y") != null && data.getExtras().get("address") != null) { String sX = data.getExtras().get("x").toString(); String sY = data.getExtras().get("y").toString(); String sAddress = data.getExtras().get("address").toString(); double dX = Double.parseDouble(sX); double dY = Double.parseDouble(sY); ShowSearch(dX, dY, sAddress); } else { Toast.makeText(this, "Error in location", Toast.LENGTH_SHORT).show(); Log.d("onActivityResult()", "Something went wrong, some extra data is null"); } } else { Toast.makeText(this, "Error in location", Toast.LENGTH_SHORT).show(); Log.d("onActivityResult()", "Something went wrong, some extra data doesn't exist"); } } else{ Toast.makeText(this, "No Location found", Toast.LENGTH_SHORT).show(); Log.d("onActivityResult()", "Something went wrong, either the result code is wrong or the data is null"); } } 
9
  • 2
    Allright, I will edit accordingly. Commented May 17, 2016 at 14:17
  • 2
    "why does Android Studio keeps giving me a warning on the try catch code" The only answer I can see to that is: Android Studio isn't doing sufficient introspection of the code to know you're handling it; or the people designing its warning system don't care whether you handle it. Commented May 17, 2016 at 14:17
  • 2
    On efficiency: The odds of it mattering are very, very, very, very low. Do whichever you feel is clearest and cleanest from a maintenance perspective. Any decent JIT would optimize the null checks pretty well in your first example (I don't know how good Dalvik's JIT is). Commented May 17, 2016 at 14:19
  • 2
    Not sure which one is more efficient, but I wouldn't catch NullPointerExceptions. When a NullPointerException is thrown it should be a programmer's error. As a programmer we make sure they won't get thrown (by null-checks). Not sure if it's just my personal preference or widely accepted by others, but I personally think catching NullPointerExceptions is bad practice and should be avoided where possible. Commented May 17, 2016 at 14:20
  • 1
    There is still something unchecked: getExtras() can return null. Use of Intent#hasExtra(String name) would solve that, idk if the nullcheck is aware of that though. Commented May 17, 2016 at 14:25

4 Answers 4

8

You should not catch a NullPointerException - in fact very few RuntimeExceptions should be caught.

A NullPointerException denotes a problem with your code, where a variable is invoked methods upon (or its fields are accessed), while the reference actually has a null value.

This essentially mandates checking for null values.

That's where Android Studio seems to be pedantically proactive in this context: of course you may get NPEs by chaining method invocations on objects, and if you do not have a guarantee that the objects will not be null, you should check for null values.

For instance:

if (resultCode == 100 && data.getExtras().get("x") != null && data.getExtras().get("y") != null && data.getExtras().get("address") != null) { ... 

... would become, tediously:

if (resultCode == 100 && data != null // unlikely && data.getExtras() != null && data.getExtras().get("x") != null ... 

... or rather, in this instance:

if (resultCode == 100 && data != null // unlikely && data.hasExtra("x") ... 

That change is tedious and adds clutter, but it will hardly matter in terms of performance, as long as your method invocations are not mutating any object (otherwise, just assign to a variable before checking for null values).

There seem to be ways to parametrize Android Studio with regards to the warnings you get from the IDE.

See this question for the general direction.

Note/Late edit

As this is an old answer, a word about Java 8's Optional.

  • Optionals are meant to convey the concept of data that may or may not be there, such as an instance of the referenced object.
  • In other words, an Optional<T> is a container for an instance of T whose presence we are not sure about.
  • The Optional class features a number of methods to deal with this uncertainty a lot more elegantly than having to tediously perform null checks, or some may argue, than having to deal with the concept of pointers at all in the first place, as conveyed by dreaded NullPointerExceptions.
  • Optionals are massively employed in Java 8´s stream API.
  • Finally, here's a good starting point on Optionals from Oracle's own point of view.
Sign up to request clarification or add additional context in comments.

4 Comments

Thank you for the informative answer, I never realised the cons of using try catches this way. However looking at your code examples, isn't it redundant to check "data != null"? Wouldn't it be fine just checking "data.getExtras().get("x") != null &&...", because if data IS null, getExtras would always return null right?
@M.Haché you're welcome. And no, it's not reduntant, precisely because if you invoke getExtras on a null Intent, you will get a NullPointerException. I'm not entirely sure whether the Intent actually can be null there, though, hence the // unlikely comment.
@M.Haché also you're better off invoking hasExtra(yourKey), instead of getExtras().get(yourKey) != null.
I tested and you are right, it WILL give an 'NullPointerException'. About using 'hasExtra(yourKey)' instead of 'getExtras().get(yourKey) != null' see this comment
1

Usually throwing an exception is something you would use to avoid unexpected programming errors or failures over different conditions. Plus, throwing exceptions can be expensive in some situations so I would definitely go with the null-checks (when possible).

Check also this question for further details.

Comments

0

I think this would suit your requirement better. Throwing exceptions is costly if your just logging it.

VariableType extrasX = data.getExtras(); VariableType extrasY = data.getExtras(); VariableType addressS= data.getExtras(); if(extrasX !=null && extraYs != null && addressS != null){ VariableType xType = extrasX.get("X"); VariableType yType = extrasY.get("Y"); VariableType addressType = addressS.get("address"); if(xType !=null && yType != null && addressType != null){ String sX = xType.toString(); String sY = yType.toString(); String sAddress = addressType.toString(); double dX = Double.parseDouble(sX); double dY = Double.parseDouble(sY); ShowSearch(dX, dY, sAddress); } else { Log.d("onActivityResult()", "Something went wrong, some data is } } else { Log.d("onActivityResult()", "Something went wrong, some data is } 

Comments

0

The accepted answer involving Optional objects requires API level 24 or higher, so I usually go with the Android Studio recommendation of inserting Objects.requireNonNull() around the highlighted object.

  • It takes away the warning
  • It leaves the possibility of failing with a NullPointerException (which leads to better code)
  • It works with lower Android API levels than Optional
  • It is much less verbose than what is required with Optional.

So instead of checking data.getExtras().get("x") != null and ignoring potential problems when it is null, it is better to use

Objects.requireNonNull(data.getExtras().get("x"))

directly in your code.

Of course, there may be situations when you are OK with it being null, in which case the data.getExtras().get("x") != null check is fine, causing the code to ignore the action when it is null.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.