assert false or RuntimeException() for unreachable code

[Gytis Jakutonis]

Hello,
sometimes we get to situation then some secions of code should never be reached. What to use: assert false or RuntimeException() for unreachable code? E.g.:

  
book()
    lock()
    try
        read()
        update()
        unlock()
    catch(RecordNotFoundException)
        // should never be reached, since we already have locked the record
        assert false;
        or
        throw new RuntimeException();
    ...

Runtime exception seems to be more obvious, since assertions may be disabled(and are disabled usually), so if ever that code becomes reachable(I can not figure out how, but who knows...), programmer(since it is programming error) will definitely get exception. Empty catch block is IMO acceptable too(and IMO should be used), but I do not include it as possible option, because Sun may be not happy to see empty catch block and deduct points without analysing a context.

[Min Huang]

you could also log a severe message

[Anonymous]

Gytis,
I'd go for RuntimeException PLUS a log entry of severe level as Min proposed. IMHO assertions should only be used during development.
Regards,
Marcel

[Javini Javono]

Hi,

book(...) {
    
    try {
        lock(); //Even if your particular version of lock() doesn't throw an
                //exception, I would see no reason not to put it in the try
                //block.  But, I'm not sure why it would not throw a record not
                //found exception.
        read();
        update();
    } catch (RecordNotFoundException rnfException) {
        //I don't think the client programmer, now coding this particular method
        //need be concerned about whether or not a particular exception can ever
        //be thrown or not;  if the compiler insists that a
        //RecordNotFoundException must be caught, then I catch it and handle it
        //accordingly (and, how it would be handled depends on what the
        //RecordNotFoundException means in this context as defined by the method
        //or methods that throw it;  presumably it has the same meaning across
        //all methods that throw the exception and can be handled in a similar
        //manner.
        //
        //In this context, a RecordNotFoundException may mean, for the layer
        //above us, that the operation failed.
        
        String message = "Unable to complete booking operation because the "
        + "record was not found."
        
        throw new BookingFailedException(message, rnfException);
    } catch (other exceptions as necessary) {
    } finally {
        try {
            if (record is locked) {
                unlock();
            }
        } catch (RecordNotFoundException rnfException) {
            //This might be indicative of a system failure of some type or a
            //logic error on somebody's part.
            
            String message = "Unlocking record = " + recordNumber + " failed.";
            throw new SystemAnomolyException(message, rnfException);
            //Or perhaps simply log it:
            log(message, rnfException);
        }
    }
}

My comments are embedded in the above code. I'm not an exceptions expert, of
course.
thanks,
Javini Javono

[Burk Hufnagel]

Hmmm... I would never use a RuntimeException there (or most places) because I don't think it's a Good Idea for the application to die just because something unexpected happened.
It's not impossible for the code you posted to get a RecordNotFound. Is it possible that a local client (that isn't using record locking) could delete the record between your lock and read or update? Unlikely perhaps, but possible.
Oddly enough the impossible can happen.
Burk

[Gytis Jakutonis]

Hi Burk,
in my implementation it is not possible for other client to delete record without holding a lock on it. So in general it is impossible to receive RecordNotFoundException after successfull lock(). Maybe file may be corrupted in some way so that record becomes unavailable, but isn't RuntimeException(or derived one) is suitable in such situation? Or should I create a bunch of checked exception for each case, e.g. LockedRecordNotFoundException etc.

[Javini Javono]

Hi,

Hi Burk,
in my implementation it is not possible for other client to delete record without holding a lock on it. So in general it is impossible to receive RecordNotFoundException after successfull lock(). Maybe file may be corrupted in some way so that record becomes unavailable, but isn't RuntimeException(or derived one) is suitable in such situation? Or should I create a bunch of checked exception for each case, e.g. LockedRecordNotFoundException etc.

Keeping in mind that I am not an exceptions expert, I hope you don't mind if I attempt to "answer"
your question; though, I certainly hope that others also respond.
Let's assume that in your code after you lock the record, your code accidentally has a line
of code which deletes the record. When you attempt to unlock the record, given your
design, this will result in a record not found exception.
Now, if everything was handled in a straightforward way, the user error message would have
a cause that the record was not found. When you further investigated, you would see that
the cause was a programmer logic error. In short, exceptions are not full proof. And, your
example is interesting in pointing out this ambiguity.
Possible solutions:
1. Test the code; this is obvious, of course. But, it will at least find these types of errors.
2. Use assertions or programmer logic error exceptions where appropriate. So, for instance,
prior to unlocking the record, you could call a new method called data.exists(recordNumber),
and if the record no longer exists, then something went wrong somewhere probably due to
a programmer logic error. [Though, I personally would not do this if the code is straightforward
and easy to understand, i.e., if the probability of a programmer logic error is small.]
3. I would not create a whole bunch of exceptions to deal with what in general appear to
be programmer logic errors.
I suspect that there is no one right answer to your interesting question and scenario you
presented. On second thought, I would revise my above initial response in a previous
posting and not throw a programmer logic error exception within the finally block,
but throw a record not found exception (and then test the code).
Thanks,
Javini Javono

[Gytis Jakutonis]

Javini,
aren't runtime exeptions used for programming errors? E.g. null pointers, index out of bounds etc. We do not expect String.substring(...) to return null if we pass invalid indexes, we expect to get runtime exception. My case is similar - I do not expect locked record to be deleted if it was locked for booking. So I get runtime exception since it is programming error.

[Burk Hufnagel]

Originally posted by Gytis Jakutonis:
Hi Burk,
in my implementation it is not possible for other client to delete record without holding a lock on it. So in general it is impossible to receive RecordNotFoundException after successfull lock(). Maybe file may be corrupted in some way so that record becomes unavailable, but isn't RuntimeException(or derived one) is suitable in such situation? Or should I create a bunch of checked exception for each case, e.g. LockedRecordNotFoundException etc.

I don't know which exam you've got. I have the Fly By Night reservation system which includes a class called Data. Data has methods to lock, unlock, modify, and delete records. As delivered there is lock and unlock are empty, and modify adn delete don't check to see if the record is locked.
Given that, a client could be written (by some other programmer) that allows the user to directly (using Data) open a database currently in use by a remote client and modify or delete any record it chose.
This client would be running in a separate JVM from the one my remote server is running and so it is possible that the database file is modified without the remote server knowing about it.
If this scenario is possible in your assignment then that code is not unreachable. I agree that it is not mentioned anywhere in the requirements for the assignment, but that does not mean it couldn't happen or that the examiner's won't try it.
Burk

[Andrew Monkhouse]

Hi Burk,
Most people here are currently working on either the "URLyBird" hotel bookings assignment, or the "Bodgitt & Scarper" building contractors assignment. There are multiple versions of each of these assignments, however they all have similar features, and can generally be compared to the Fly By Night Services assignment.
One major difference between the new assignments and the FBNS is that the Data class is no longer provided - candidates must now write the entire Data class, making it implement a provided interface.
So it is quite possible that a current candidate could write the update and delete methods such that locking is mandatory. In fact, depending on the individual candidates instructions, this may even be mandatory.
Under such conditions, it would not be possible for a client to modify a record without first gaining the lock.
Your alternate suggestion of a rouge process deleting the record is also not possible under the new instructions, which states "You may assume that at any moment, at most one program is accessing the database file".
Regards, Andrew