1

Lets say I have following class:

public class RectangularPrism{
    public Integer width;
    public Integer height;
    public Integer depth;

    //setters and getters
}

I use it for calculate sum of volume of all prisms. I have two methods to calculate this. Which one is better?

This one use conditional operator.

public Integer volumeSum(List<RectangularPrism> prismList){
        Integer sum = 0;
        for(RectangularPrism prism: prismList){
            Integer width = prism.getWidth() == null ? 0 : prism.getWidth();
            Integer height = prism.getHeight() == null ? 0 : prism.getHeight();
            Integer depth = prism.getDepth() == null ? 0 : prism.getDepth();
            sum += width * height * depth;
        }
        return sum;
    }

This one use try catch block

public Integer volumeSum(List<RectangularPrism> prismList){
        Integer sum;
        for(RectangularPrism prism: prismList){
            try {
                Integer width = prism.getWidth();
                Integer height = prism.getHeight();
                Integer depth = prism.getDepth();
                sum += width * height * depth;
            }catch( NullPointerException e){}
        }
        return sum;
    }

Which one would be better If my class would have 100 or more fields which determine result?

Is it fine to use try catch to handle null values?

Svante
  • 50,694
  • 11
  • 78
  • 122
nervosol
  • 1,295
  • 3
  • 24
  • 46

3 Answers3

13

This is a bad idea on every level.

  1. try-catch is actually slower (if there really is an exception thrown)
  2. You're using exceptions in a non-exceptional case (i.e. normal control flow), which is conceptually wrong.
  3. If anything else in the try-catch block throws a legitimate NPE, you have no way of telling what happened. (Imagine the case when prism itself is null for example.)
  4. It is very difficult to figure out from the code alone what your intent was. Using the ?: operator to check for nulls is a well-established pattern that everyone will recognise.
  5. An empty catch block is a big no-no, and probably the most reliable method of generating bugs that will take ages to track down.
  6. In this case the two snippets aren't even equivalent, as in the "exception-driven" one a null value will mean the rest of the try block will be skipped. Of course the sum will still be correct but that's just a coincidence.

If you really want to improve performance, don't use Integer and other wrapping classes for simple calculations, use the corresponding primitive (int in this case) instead. The added bonus of this is that you suddenly won't have to worry about nulls any more because primitive values can never be null.

So to sum it up: never ever do this.

Seriously, don't. Every time you do it, a cute little kitten dies. enter image description here

Community
  • 1
  • 1
biziclop
  • 48,926
  • 12
  • 77
  • 104
  • Maybe the only "valid" ussage of `try...catch` here would be to handle a null `prism`... other than that I completely agree (but I don't mind if a few kittens die :P ) – Barranka Sep 24 '14 at 16:39
3

You should never use try-catch to handle the control flow of your program.

Always use the established methods, in this case, check for null as you do in your first code sample.

Nimelrian
  • 1,726
  • 13
  • 23
0

I know this will not answer the question (Can try .. catch be used for code optimization), but I think you should rarely bother with null values: you should enforce the (width, height, depth) to be non null if they are intrinsic property of your object.

I mean, you are building a RectangularPrism: width, height, and depth define this object. You can't have null in there because otherwise it would not be a rectangular prism.

In that case, I would rather do that:

public class RectangularPrism {
    private final Integer width;
    private final Integer height;
    private final Integer depth;    
    public RectangularPrism(Integer width, Integer height, Integer depth) {
      this.width = Objects.requireNonNull(width, "width");
      this.height = Objects.requireNonNull(height, "height");
      this.depth = Objects.requireNonNull(depth, "depth");
    }
    // getters    
}

Or with setter:

public class RectangularPrism {
    public Integer width;
    public Integer height;
    public Integer depth;

    public void setWidth(Integer width) {
      this.width = Objects.requireNonNull(width, "width");      
    }
}

You don't have to handle null because the JVM will throw a NullPointerException, but only because the object was not fully/correctly initialized, not because you did not handle a null value that should not be there. This is a normal behavior like throwing an IllegalStateException because the object is in a state it should not be (here: because you have null values).

And to be complete, you would have a better implementation if the volume computation were an operation on RectangularPrism rather than an external operation.

This way would be more correct:

public class RectangularPrism {
    private final Integer width;
    private final Integer height;
    private final Integer depth;    
    public RectangularPrism(Integer width, Integer height, Integer depth) {
      this.width = Objects.requireNonNull(width, "width");
      this.height = Objects.requireNonNull(height, "height");
      this.depth = Objects.requireNonNull(depth, "depth");
    }
    public Integer computeVolume() {
      return width * height * depth; // can't be null.
    }
}

Then (I used int there, because there is a sum, I won't comment the use of primitive over wrapper class):

public Integer volumeSum(List<RectangularPrism> prismList) {
  int initial = 0;
  for (RectangularPrism p : prismList) 
    initial += p.computeVolume();
  return initial;
}

And last but not least, exception handling is slower, but exception handling in a loop is way slower.

NoDataFound
  • 11,381
  • 33
  • 59