6

Here, my main goal is setting the value safely, without having a performance (speed, memory, cpu etc) impact.

I have a silly option (in a bad style) also mentioned below. So, what is the best way to do this? option 1? option 2? or another one?

Option 1 :

if(
    animalData!=null && 
    animalData.getBreedData()!=null && 
    dogx.getBreed() != null && dogx.getBreed().getBreedCode() != null && 
    animalData.getBreedData().get(dogx.getBreed().getBreedCode()) != null
){
    dogx.getBreed().setBreedId(animalData.getBreedData().get(dogx.getBreed().getBreedCode()));
}

Option 2 :

try{dogx.getBreed().setBreedId(animalData.getBreedData().get(dogx.getBreed().getBreedCode()));}catch(Exception e){}

Note : this piece of code is in a loop having many thousands of iterarations.

9
  • 3
    And why don't you try to avoid having to trigger an NPE in the first place? Commented May 29, 2017 at 13:29
  • 3
    This question seems to be opinion based. The rule of thumb though is to use exceptions for exceptional circumstances. Do you expect it to fail often or rarely? Commented May 29, 2017 at 13:29
  • 1
    There is a lot more to this question than the overhead, because only one solution matches intentions of Java language designers. Voting to reopen. Commented May 29, 2017 at 13:30
  • 2
    Also see stackoverflow.com/questions/36343209/…. As a side note, try {......} catch(Exception e) {} is pretty terrible. You should at least catch NullPointerException since that's the only one you're interested in. Commented May 29, 2017 at 13:57
  • 1
    All you are really doing when you test for null is checking the value of a pointer. It's basically the same as n != 0. Java just hides that detail from you. The virtual method calls are probably more expensive, because they read from shared memory where the null checks essentially read from a local variable. Commented May 29, 2017 at 14:08

3 Answers 3

8

Checking for nulls is the only option that is consistent with Java exception philosophy.

NullPointerException is a RuntimeException, which means that it is designed for reporting programming errors. This makes it an extremely bad practice to use it for anything other than terminating your program.

You can optimize your code by storing references to objects for null-checking, so that you can reuse them later:

BreedData breedData;
DogBreed dogBreed;
String breedCode;
String breedId;
if( animalData != null
&&  (breedData = animalData.getBreedData())!=null
&&  (dogBreed = dogx.getBreed()) != null
&&  (breedCode = dogx.getBreed().getBreedCode()) != null
&&  (breedId = breedData.get(breedCode)) != null
) {
    dogBreed.setBreedId(breedId);
}
Sign up to request clarification or add additional context in comments.

5 Comments

So then how can you assure that this solution is faster than catching the NPE exception.
@wero OP presented only one solution that is valid. The other one is not valid, even if it is faster (which could very well happen - for example, when there are no exceptions for all objects in the loop).
What is valid? Consistent with your philosophy? Or accepted by the compiler?
wero and dasblinkenlight : Here's nice argument is going on. I have small addon here. How about the resource consumption by guarding a statement with try-catch? Although there's no exceptions for the loop will it consume additional resources because of guard them using try-catch?
@namalfernandolk There isn't much resources taken by setting up and tearing down a try/catch. The real cost comes into play when the exception is actually thrown. If your loop repeats the call, say, 1,000,000 times, and only two or three of these repetitions result in an exception handler, your second option may finish a tiny bit faster. If, on the other hand, some 10,000 items end up in an exception handler, the first option will probably be significantly faster.
3

Option 3:

Optional.ofNullable(animalData)
    .map(animalData -> animalData.getBreedData())
    .ifPresent(breedData -> {
        Optional.ofNullable(dogx.getBreed())
            .map(breed -> breed.getBreedCode())
            .ifPresent(breedCode -> {
                thisBreedData = breedData.get(breedCode); // here we could use a third Optional call…
                if (thisBreedData != null) {
                    dogx.getBreed().setBreedId(thisBreedData));
                }
            }) 
    });
}

3 Comments

...without having a performance (speed, memory, cpu etc) impact...
@wero Is there a significant impact with my solution?
yes, it will be slower, consume more memory and cpu time besides having more lines of code than the original solutions
0

Above answers don't actually answer your question. So here is my:

In case performance is really matters for you - removing null checks may be a good idea. Many high-performance libraries use this approach, for example, here is code of FastList class from HikariCP (fastest java DB connection pool):

   public boolean add(T element)
   {
      try {
         elementData[size++] = element;
      } catch (ArrayIndexOutOfBoundsException e) {
          ...
      }
}

This try catch was added as replacement of range check. Removal of bounds-checks actually makes this code faster.

Here is the benchmark that proves that:

@BenchmarkMode(Mode.Throughput)
@Fork(1)
@State(Scope.Thread)
@Warmup(iterations = 10, time = 1, timeUnit = TimeUnit.SECONDS, batchSize = 1000)
@Measurement(iterations = 20, time = 1, timeUnit = TimeUnit.SECONDS, batchSize = 1000)
public class BoundsCheck {

    @Param("1")
    int index;
    int[] ar;

    @Setup
    public void setup() {
        ar = new int[] {1, 2};
    }

    @Benchmark
    public int boundCheck() {
        if (index >= ar.length) {
           throw new IndexOutOfBoundsException();
        }
        return ar[index];
    }

    @Benchmark
    public int noCheck() {
        return ar[index];
    }

    @Benchmark
    public int noCheckWithTryCatch() {
        try {
            return ar[index];
        } catch (RuntimeException e) {
            return -1;
        }
    }
}

Result:

Benchmark                        (index)   Mode  Cnt       Score       Error  Units
BoundsCheck.boundCheck                 1  thrpt   20  334197.761 ±  2593.034  ops/s
BoundsCheck.noCheck                    1  thrpt   20  370148.527 ±  9016.179  ops/s
BoundsCheck.noCheckWithTryCatch        1  thrpt   20  371782.744 ± 17290.347  ops/s

What can we see here? Eliminating of bound-check gives you +10% performance gain. try {} catch costs nothing until exception is thrown.

However, this approach is suitable only in situations when you can guarantee that you'll have no NPE's in your data. So exceptions actually will never be thrown or thrown very rarely. Otherwise, exception throwing may make your code even slower.

Here is no precise answer. This really depends on your data and you need to know your data in order to make any further conclusions.

Also, have in mind that JIT can take care of eliminating null's checks in your code when it can, so this optimization may not worth it. Only tests with real data could give you an answer.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.