views:

138

answers:

6

I doubt a definite answer exists but I'm quite interested in different opinions on the subject. A communiti wiki therefore.

You're designing a method. It serves a purpose, that's why you're designing it in the first place. A caller uses your method and the method fails, but, lo and behold, the ultimate purpose that exuse the existence of the method is nevertheless achieved -- thanks to external circumstances that are out of your control or some kind of magic, you choose. Would you report this situation to the caller as a failure or as a success?

Let's pick a trivial example. You're writing a DeleteFile function. It takes a file path and deletes the file. Someone calls this function, providing a path. The function looks the file up, but it does not exist. It's not a permissions issue or something, the file is genuinely missing. Maybe another process deleted it a microsecond ago, and maybe it never existed at all. The function failed to perform its task, so it must report a failure... but the only reason why a user whould call this function is to make sure a file does not exist, and, voila, it does not, so it is a success.

I anticipate answers like "just be more verbose in your return values", and I'm happy to return a verbose result as a complimentary information, but what (and why) would you return as a primary success flag of the function, success or failure? Would it be FALSE with additional flag of BUT_DOES_NOT_EXIST_ANYWAY, or would it be TRUE with a BUT_THANK_SOMEONE_ELSE flag?

EDIT

Please do not give answers that only apply to the example above. I'm asking about situation in general, including when the method has no parameters or it is not possible to call it in a wrong way for some other reason.

+6  A: 

Follow the principle of least surprise, and throw an exception.

The way I think of it is this: If I were writing code and included a bug in my code that passed an incorrect path to the DeleteFile function, a path so incorrect that it could never exist, I would expect the DeleteFile to throw, to indicate that I had done something wrong.

If you want you can create an overload that accepts a param called ThrowOnFileNotExist or something, and if set to false, it would not throw in this situation. But I'd think that would be the rare outlier case.

Cheeso
Very good point. Yet I'm more interested in situations where providing an invalid parameter to the method is not an issue. Like, imagine a method that does not have parameters at all.
GSerg
+4  A: 

That's what exceptions are for.

throw new FileDoesNotExistException

Rich
+12  A: 

When new programmers are learning how to write functions, the terms preconditions and postconditions are often introduced. Preconditions are assumptions that must be met for your method to work properly. Postconditions are guarantees that your method makes about the state of the system after execution.

I think that a reasonable precondition to expect when calling a deleteFile method is that the file currently exists. If the preconditions are not met, the method should fail, even if the postconditions have been achieved in an accidental, roundabout way.

Now, if your method were called deleteFileIfExists, that's a different story. You haven't imposed such strict preconditions on the method.

Mike Daniels
+3  A: 

If you set out to fail, and succeed in doing so, what have you done?

I think it's up to the client to decide what to do. If your function is asked to delete a file that doesn't exist, throw an exception. The client can then catch that exception and decide that whether the file didn't exist or it was deleted successfully, the end result is the same and it can carry on normally.

Alternatively, if the file missing is an expected possibility, i.e. not exceptional, you could just return a failure status which the caller is free to ignore :) That again leaves the decision to the client.

In either case, if you follow the principle that a function should do one thing and do it well, the client can string function calls together in any appropriate manner for the task at hand, and success or failure becomes much more finely grained.

What you should not do is return success if the function did not complete successfully. It doesn't matter if the end result is coincidentally the same. That way madness lies.

Duncan
A: 

The function should throw an exception (or return a fail value, whatever). The user, though, should probably see success.

IOW, programmers calling the function should know the truth. But what the user interface presents to the user rather depends on the overall functionality, and I suppose the target user.

To take your "delete file" example -- under normal circumstances I would expect your application to just remove the file from the list, or whatever, and move on -- let the user see the file has been deleted; they probably don't care how it happened.

("under normal circumstances" because in some applications, it might be important.)

Shadowfirebird
As in, the user selects a file and says delete. The programmer calls DeleteFile(file), and handles the exception FileDoesNotExist by removing the file from the list?
s_hewitt
Broadly speaking. What I'm saying is that the programmer that calls the function might care about the difference even if the user of his program does not. Ideally, no function should assume it knows anything about the code that calls it. Maybe the function is being used in a completely different piece of code to the one it was written for!
Shadowfirebird
+1  A: 

Your example is a perfect one for the ternary boolean: True/False/FileNotFound.

enum Bool 
{ 
    True, 
    False, 
    FileNotFound 
};

UPDATE
All joking aside. If the method is unable to perform it's function then it should have a return code (or exception thrown) appropriate to the situation. It doesn't matter to the method if the end result is the same; although it might to the caller of that method.

I prefer to avoid throwing exceptions and instead maintain return codes. IMHO Exception types exist for situations that code can not handle. Your example method is small enough that simply returning False is good enough. It would be up to the calling code to determine why.

There are MANY reasons why a simple method like DeleteFile could fail. File doesn't exist, you don't have security rights, file is locked, file system is being retarded, etc. Each of these should have their own specific return failure code.

Then it's up to the calling code to decide "oh file doesn't exist, never mind". Or, "What do you mean I don't have rights?!? Abort!!"

Point is: Don't return true unless the method can actually execute.

Chris Lively
Um ... how do you represent ternary state with a bit? Or a voltae for that matter?
Duncan
Follow the link. It's a *long* running joke in some circles.
Chris Lively