Affect of Implicity vs. Explicit Exception Handling to the Code Quality

In this post I will try to compare 3 different implementations of the same method which are equivalent with respect to the set of unit tests they are expected to pass. The comparison is mainly based on the way exceptions are handled in each and I am comparing them with respect to how they impact code quality.

Here is the method we are expected to implement:





/**
* Should push given element to stack with given stack no.
* Should throw a RuntimeException if the capacity is full.
**/
public void push( int stackNo, T element ) {
...
}



A part of the unit test that is written for this method is as follows:




// Available capacity at this point is 2:
ms.push( 10, e1);
ms.push( 10, e1); //becomes full at this point.
try {
ms.push( 10, e1);
assertTrue( false );
} catch( RuntimeException e ) {
assertTrue( true );
}


Here are 3 alternative implementations that pass the tests:

1)




public void push( int stackNo, T element ) {
elements[num_elements] = (StackRecord) new StackRecord();
elements[num_elements].element = element;
elements[num_elements++].stackNo = stackNo;
}



This implementation works. In fact when there is no more capacity of the elements array (num_elements >= elements size), it will throw an ArrayIndexOutOfBounds Exception. This exception is already a RuntimeException so there is no problem.



2)




public void push( int stackNo, T element ) {
try {
elements[num_elements] = (StackRecord) new StackRecord();
elements[num_elements].element = element;
elements[num_elements++].stackNo = stackNo;
} catch (RuntimeException e) {
throw new RuntimeException ();
}
}



This time we catch any possible exception and then throw an exception again.


3)




public void push( int stackNo, T element ) {
if ( num_elements >= elements.length ) // full capacity
throw new RuntimeException( "capacity is full" );
else {
elements[num_elements] = (StackRecord) new StackRecord();
elements[num_elements].element = element;
elements[num_elements++].stackNo = stackNo;
}
}



This time we check a condition and then throw an exception when the condition is true.


Let's compare all three.
In terms of passing the tests, all 3 are ok.
However, in terms of quality of the code, the first 2 approaches have problems.

The first one has the exception thrown implicitly. So anybody looking at this code won't be easily able to tell you that the code throws an exception and when it will throw an exception.

The second one tells you that an exception is thrown in some cases. However, it is not clear why an exception is caught and then thrown again, and most importantly it is not clear in what causes the exception to be thrown in the first place.

IMHO, the last approach has the best among these 3 in terms of source code quality. It explicitly states the condition that signals an error condition and then it again explicitly throws the exception.

Comments

Popular posts from this blog

Accessing Resources in Java, use of getResource method of Class and ClassLoader, How to access test data in unit tests

The Order of Rails Validation Filters

Comparison of equality operators in Ruby: == vs. equal? vs. eql? vs. ===