tags:

views:

94

answers:

6

Which of the following ways of handling this precondition is more desirable and what are the greater implications?

1:

If Not Exists(File) Then 
    ThrowException
    Exit
End If
File.Open
...work on file...

2:

If Exists(File) Then 
    File.Open 
    ....work on file...
Else
    ThrowException
    Exit
End

Note: The File existence check is just an example of a precondition to HANDLE. Clearly, there is a good case for letting File existence checks throw their own exceptions upwards.

+1  A: 

It's a style thing. Both work well however I prefer option 1. I like to exit my method as soon as I can and have all the checks up front.

Dan
+3  A: 

I prefer the first variant so it better documents that there are preconditions

dfa
+1  A: 

Readability of first approach is higher than the second one.
Second option can nest quite fast if you have several preconditions to check; moreover, it suggests that the if/else is somehow in the normal flow, while it is really only for exceptional situations.

As well, expressiveness of first approach is therefore higher than the second one.
As we are talking about preconditions, they should be checked in the beginning of the procedure, just to ensure the contract is being respected; for this reason, the entire check should be somehow separated from the remaining part of the procedure.

For these two reasons, I would definitely go for the first option.


Note: I am talking here about preconditions: I expect that the contract of your function explicitly defines the file as existing, and therefore not having it would be a sign of programming error.
Otherwise, if we are simply talking about exception handling, I would simply leave it to the File.Open, handling that exception only if there is some idea on how to proceed with that.

Roberto Liffredo
+1  A: 

Every exception must be produced at the appropriate level. In this case, your exception is an open() issue, which is handled by the open() call. Therefore, you should not add exception code to your routine, because you would duplicate stuff. This holds unless:

  1. you want to abstract your IO backend (say your high level routine can either use file open, but also MySQL in the future). In this case, it would be better for client codes to know a more standard and unique exception will be produced if IO issues arise
  2. the presence of a low level exception implies a higher level exception with high level semantic (for example, not being able to open a password file means that no auth is possible and therefore you should raise something like UnableToAuthenticateException)

As for coding style of your two cases, I would definitely go for the first. I hate long blocks of code, in particular under ifs. They also tend to nest and if you choose the second strategy, you will end up indenting too much.

Stefano Borini
+2  A: 

Separating the pre-condition check from work is only valid if nothing can change between the two. In this case an external event could delete the file before you open it. Hence the check for file existence has little value, the open call has to check this anyway, let it produce the exception.

djna
+1  A: 

A true precondition is something which, if happens, is a bug in the caller situation: you design a function under certain conditions but they are not hold, so the caller should never have called the function with these data.

Your case of not finding a file could be like this, if the file is required and its existence is checked before in another part of the code; however, this is not quite so, as djna says: file deletion or network failure could cause an error to happen right when you open the file.

The most common treatment is then to try to open the file, and throw an exception on failure. Then, assuming that an exception hasn't been thrown, continue with normal work.

Daniel Daranas