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.

ironwood
  • 8,936
  • 15
  • 65
  • 114
  • 3
    And why don't you try to _avoid_ having to trigger an NPE in the first place? – fge May 29 '17 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? – Carcigenicate May 29 '17 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. – Sergey Kalinichenko May 29 '17 at 13:30
  • @fge:according to the implementation and design, there is a possibility of unavailability of data for these like optional fields. So, there's a possibility of having NPE. eg : for some dogs, breedCode will not be available. – ironwood May 29 '17 at 13:38
  • @Carcigenicate : Currently there's a high chance of failing since this is going to be a new feature. In future the fail ratio will be less. – ironwood May 29 '17 at 13:51
  • 2
    Also see https://stackoverflow.com/questions/36343209/which-part-of-throwing-an-exception-is-expensive. 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. – Radiodef May 29 '17 at 13:57
  • Thank you a lot @Radiodef for the point and the reference! It is a very valuable resource. But I'm still thinking of this on the point that it must go each and every condition in a ideal no-exception situation also. won't be a performance impact? how to avoind that? – ironwood May 29 '17 at 14:03
  • 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. – Radiodef May 29 '17 at 14:08
  • Yep! got your point. Thank you @Radiodef. – ironwood May 30 '17 at 05:10

3 Answers3

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);
}
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • So then how can you assure that this solution is faster than catching the NPE exception. – wero May 29 '17 at 13:41
  • @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). – Sergey Kalinichenko May 29 '17 at 13:51
  • What is valid? Consistent with your philosophy? Or accepted by the compiler? – wero May 29 '17 at 13:55
  • 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? – ironwood May 29 '17 at 13:58
  • 3
    @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. – Sergey Kalinichenko May 30 '17 at 01:45
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));
                }
            }) 
    });
}
glglgl
  • 89,107
  • 13
  • 149
  • 217
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.

Dmitriy Dumanskiy
  • 11,657
  • 9
  • 37
  • 57