Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I have been thinking about this issue for a while and I would be curious to have opinions from other developers.

I tend to have a very defensive style of programming. My typical block or method looks like this:

T foo(par1, par2, par3, ...)
{
    // Check that all parameters are correct, return undefined (null)
    // or throw exception if this is not the case.

    // Compute and (possibly) return result.
}

Also, during the computation, I check all pointers before dereferencing them. My idea is that, if there is some bug and some NULL pointer should appear somewhere, my program should handle this nicely and simply refuse to continue the computation. Of course it can notify of the problem with an error message in the log or some other mechanism.

To put it in a more abstract way, my approach is

if all input is OK --> compute result
else               --> do not compute result, notify problem

Other developers, among them some colleagues of mine, use another strategy. E.g., they do not check pointers. They assume that a piece of code should be given correct input and it should not be responsible for what happens if the input is wrong. Also, if a NULL pointer exception crashes the program, a bug will be found more easily during testing and have more chances of being fixed.

My answer to this is normally: but what if the bug is not found during testing and appears when the product is already being used by the customer? What is a preferred way for the bug to manifest itself? Should it be a program that does not perform a certain action, but can still continue to work, or a program that crashes and needs to be restarted?

Summarizing

Which of the two approaches to handling wrong input would you advise?

Inconsistent input --> no action + notification

or

Inconsistent input --> undefined behaviour or crash

Edit

Thanks for the answers and suggestions. I am a fan of design by contract too. But even if I trust the person who has written the code calling my methods (maybe it is myself), there can still be bugs, leading to wrong input. So my approach is to never assume a method is passed correct input.

Also, I would use a mechanism to catch the problem and notify about it. On a development system, it would e.g. open a dialog to notify the user. In a production system it would just write some information to the log. I do not think that extra checks can lead to performance problems. I am not sure if assertions are enough, if they are switched off in a production system: maybe some situation will occur in production that had not occured during testing.

Anyway, I was really surprised that many people follow the opposite approach: they let the application crash "on-purpose" because they maintain that this will make it easier to find bugs during testing.

share|improve this question
    
Always code defensively. Eventually, for performance reason, you can put a switch to disable some tests in release mode. –  deadalnix Aug 25 '11 at 0:22
    
Today I have fixed a bug related to a missing NULL pointer check. Some object got created during application log out and the constructor used a getter to access some other object that was not there any more. The object was not meant to be created at that point. It was created due to another bug: some timer was not stopped during log out --> a signal was sent --> recipient tried to create an object --> constructor queried and used another object --> NULL pointer --> crash). I really would not like that such a nasty situation crashes my application. –  Giorgio Sep 8 '11 at 11:04
1  
Rule of Repair: When you must fail, fail noisily and as soon as possible. –  deadalnix Sep 8 '11 at 11:56
    
"Rule of Repair: When you must fail, fail noisily and as soon as possible.": I guess all those Windows' BSOD's are an application of this rule. :-) –  Giorgio Oct 5 '12 at 8:55

6 Answers 6

up vote 6 down vote accepted

You've got it right. Be paranoid. Don't trust other code, even if it's your own code. You forget things, you make changes, code evolves. Don't trust outside code.

A good point was made above: what if the inputs are invalid but the program does not crash? Then you get garbage in the database and errors down the line.

When asked for a number (e.g. price in dollars or number of units) I like to enter "1e9" and see what the code does. It can happen.

Four decades ago, getting my B.S. in Computer Science from U.C.Berkeley, we were told that a good program is 50% error handling. Be paranoid.

share|improve this answer
    
Yes, IMHO this is one of the few situations in which being paranoid is a feature and not a problem. –  Giorgio Aug 25 '11 at 8:25
    
"What if the inputs are invalid but the program does not crash? Then you get garbage in the database and errors down the line.": Instead of crashing the program could refuse to perform the operation and return an undefined result. Undefined will propagate through the computation and no garbage will be produced. But the program need not crash to achieve this. –  Giorgio Sep 12 '11 at 17:41
    
Yes, but - my point is that the program must DETECT the invalid input, and cope with it. If the input is not checked, it will work it's way into the system and nasty things come later. Even crashing is better than that! –  Andy Canfield Sep 30 '11 at 8:45
    
I totally agree with you: my typical method or function starts with a sequence of checks to make sure that the input data is correct. –  Giorgio Sep 30 '11 at 11:21
    
Today I had again a confirmation that the "check everything, trust nothing" strategy is often a good idea. A colleague of mine had a NULL pointer exception because of a missing check. It turned out that in that context it was correct to have a NULL pointer because some data had not been loaded, and it was correct to check the pointer and simply do nothing when it is NULL. :-) –  Giorgio Oct 14 '11 at 9:01

You already have the right idea

Which of the two approaches to handling wrong input would you advise?

Inconsistent input --> no action + notification

or better

Inconsistent input --> appropriately handled action

You can't really take a cookie cutter approach to programming (you could) but you'd end up with a formulaic design which does things out of habit rather than from conscious choice.

Temper dogmatism with pragmatism.

Steve McConnell said it best

Steve McConnell pretty much wrote the book (Code Complete) on defensive programming and this was one of the methods that he advised that you should always validate your inputs.

I don't recall if Steve mentioned this, however you should consider doing this for non-private methods and functions, and only others where deemed necessary.

share|improve this answer
2  
Instead of public, I would suggest, all non-private methods to defensively cover languages that have protected, shared, or no concept of access restriction (everything is public, implicitly). –  JustinC Aug 25 '11 at 4:44

There's no "correct" answer here, particularly without specifying the language, the type of code, and the type of product that the code might go into. Consider:

  • Language matters. In Objective-C, it's often okay to send messages to nil; nothing happens, but the program doesn't crash, either. Java doesn't have explicit pointers, so nil pointers aren't a big concern there. In C, you need to be a little more careful.

  • To be paranoid is to unreasonable, unwarranted suspicion or mistrust. That's probably not any better for software than it is for people.

  • Your level of concern should be commensurate with the level of risk in the code and the probable difficulty of identifying any problems that do show up. What happens in the worst case? The user restarts the program and continues where they left off? The company loses millions of dollars?

  • You can't always identify bad input. You can religiously compare your pointers to nil, but that only catches one out of 2^32 possible values, nearly all of which are bad.

  • There are a lot of different mechanisms for dealing with errors. Again, it depends to some degree on the language. You can use assert macros, conditional statements, unit tests, exception handling, careful design, and other techniques. None of them is foolproof, and none is appropriate for every situation.

So, it mostly boils down to where you want to put responsibility. If you're writing a library for use by others, you probably want to be as careful as you reasonably can about the inputs you get, and do your best to emit helpful errors when possible. In your own private functions and methods, you might use asserts to catch silly mistakes but otherwise put responsibility on the caller (that's you) to not pass garbage in.

share|improve this answer
    
+1 - Good answer. My main concern is that a wrong input can cause a problem that appears on a production system (when it is too late to do something about it). Of course, I think you are totally right in saying that it depends on the damage such a problem can cause to the user. –  Giorgio Aug 25 '11 at 7:18

There should definitely be a notification, such as a thrown exception. It serves as a heads up to other coders who may be misusing code you wrote (trying to use it for something it wasn't intended to do) that their input is invalid or results in errors. This is very useful in tracking down errors, whereas if you simply return null, their code will continue along until they try to use the result and get an exception from different code.

If your code encounters an error during a call to some other code (perhaps a failed database update) which is beyond the scope of that particular piece of code, you really have no control over it and your only recourse is to throw an exception explaining what you know (only what you are told by the code you've called). If you know that certain inputs will inevitably lead to such a result, you can just not bother executing your code and throw an exception stating which input is not valid and why.

On a more end-user related note, it is best to return something descriptive yet simple so that anyone can understand it. If your client calls and says "the program crashed, fix it", you have a lot of work on your hands tracking down what went wrong and why, and hoping you can reproduce the problem. Using proper handling of exceptions can not only prevent a crash, but provide valuable information. A call from a client saying "The program is giving me an error. It says 'X Y Z is not valid input for method M, because Z is too large", or some such thing, even if they have no idea what it means, you know exactly where to look. Additionally, depending on your/your company's business practices, it might not even be you fixing these problems, so it is best to leave them a good map.

So the short version of my answer is that your first option is the best.

Inconsistent input -> no action + notify caller
share|improve this answer

I struggled with this same issue while going through a university class in programming. I leaned towards the paranoid side and tend to check everything but was told that this was misguided behavior.

We were being taught "Design by contract". Emphasis is that the preconditions, invariants and post-conditions be specified in the comments and design documents. As the person implementing my part of the code, I should have trust in the software architect and empower them by following the specifications which would include the preconditions (what inputs my methods have to be able to handle and what inputs I will not be sent). Excessive checking in every method call results in bloat.

Assertions should be used during build iterations to verify program correctness (validation of preconditions, invariants, post conditions). Assertions would then be turned of in the production compile.

share|improve this answer

Using "assertions" is the way to go to notify fellow developers that they are doing it wrong, in "private" methods only of course. Enabling/disabling them is just one flag to add/remove at compile time and as such it is easy to remove assertions from the production code. There are also a great tool to know if you are somehow doing it wrong in your own methods.

As for verifying input parameters within public/protected methods, I prefer to work defensively and check parameters and throw InvalidArgumentException or the like. That is why there are here for. It also depends on whether you are writing an API or not. If it's an API, and even more if it is closed source, better validate everything so that the developers know precisely what went wrong. Otherwise, if the source is available to other developers, it is not black/white. Just be consistent with your choices.

Edit: just to add that if you look for example at the Oracle JDK, you will see that they never check for "null" and let the code crash. Since it is going to throw a NullPointerException anyway, why bother checking for null and throwing an explicit exception. I guess it makes some sense.

share|improve this answer
    
In Java you get a null pointer exception. In C++ a null pointer crashes the application. Maybe there are other examples: division by zero, index out of range, and so on. –  Giorgio Aug 25 '11 at 6:54
    
Thanks for the correction. –  Jalayn Aug 25 '11 at 7:11

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.