The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Throwing an exception after contract was broken

Suppose that class A has method B(int), whose contract says "call me with arguments > 0". Do you agree with me that it doesn't make sense to throw an exception after someone invokes A::B(-1)?

What would you do instead?
quant dev Send private email
Sunday, October 26, 2008
 
 
I would return a 'zero' value or some such.  The idea that every 'contract' should be honored or an execption will be thrown is equivalent to, imo, Java checked exceptions.  So, if you just want to see a bunch of empty try blocks in your code, sure, throw exceptions.  Otherwise, try to fail gracefully.
.02
Sunday, October 26, 2008
 
 
What I do is put assertions in the method and then assume that the caller honours the contract.

The only exception I'd make is that silently accepting incorrect parameters might cause (in C/C++) a buffer overflow, which is evil. I think that terminating the program would be a good solution in that case.
quant dev Send private email
Sunday, October 26, 2008
 
 
Yes, you should throw an exception or not have the constraint. If you can handle a negative parameter then why even specify the parameter constraint of >0?

For example, if I had a method, Divide( int numerator, int denominator) and they passed in 0 for the denominator, I'd throw an exception since you can't divide by 0. But if I had a FindById(int id) and all id's are positive I wouldn't throw an exception if a -1 was passed in (nor would I have a constraint), I would just treat it the same way I would treat it if they had passed in a positive id which wasn't a valid id, etc.

Sunday, October 26, 2008
 
 
In the likely case of Divide returning a double, I'd return Inf, -Inf or NaN.
quant dev Send private email
Sunday, October 26, 2008
 
 
The situation is really a trade off. Many libraries provide two functions to have a version of a function that throws an exception if the input is invalid, and a version that doesn't. Compare for example Double.Parse[1] and Double.TryParse[2] in the .NET framework, or Python's string find[3] and index[4] functions.

When you're writing the function, you need to think about what a sensible way to handle the return value for invalid arguments is. The only real general pattern I can suggest is to use the sample function style as TryParse() - return a boolean value indicating whether the arguments are valid, and have a reference parameter by which the result is returned.

There are other things you could do - return -1 as many IndexOf() implementations do, but it really depends on what values fall into the range of your function's "normal" response values. Because Parse()'s normal return values span the whole of the double space, no value can be used to indicate an invalid argument. IndexOf()'s range is only the non-negative integers, so -1 is available as a special value.

[1] http://msdn.microsoft.com/en-us/library/fd84bdyt.aspx
[2] http://msdn.microsoft.com/en-us/library/994c0zb1.aspx
[3] http://docs.python.org/library/stdtypes.html#str.find
[4]http://docs.python.org/library/stdtypes.html#str.index

Sunday, October 26, 2008
 
 
> Do you agree with me that it doesn't make sense to throw an exception after someone invokes A::B(-1)?

No; throwing an exception, if you can, seems like the most obvious thing to do. Why do you think it doesn't make sense?
Christopher Wells Send private email
Monday, October 27, 2008
 
 
Because either they are caught, which doesn't make sense as the caller can validate the input instead of using try/catch block, or they are not, in which case I can stop the execution of the program using more clear means (like, abort()).
quant dev Send private email
Monday, October 27, 2008
 
 
> Because ...

I see.

I suppose an argument for exceptions is that for example if I'm writing the high-level program which uses the low-level class A, then I'd like to be the one who chooses whether and how my program will handle exceptions: e.g. I may prefer to not use any low-level class which may decide unilaterally to call abort().

> doesn't make sense as the caller can validate the input instead of using try/catch block

Perhaps there are many places where input ought to be or could be validated, to be contrasted with just a single high-level place where a try/catch block could be placed.

In fact if the low-level class will promize to validate input and throw an exception if it's bad, maybe I'll choose to delegate my input-validation to your low-level class: i.e. instead of me validating input *as well as* you, I'll let you validate the input *instead of* me.

> stop the execution of the program using more clear means (like, abort()).

Also I don't see that "abort" is any "clearer" than an unhandled exception.
Christopher Wells Send private email
Monday, October 27, 2008
 
 
I would generally throw an exception on invalid arguments, although I suppose it depends on what your function does.

If it is used to select a colour scheme, you should just return the default if you get invalid arguments, but the "contract" would cover this.

OTOH, if the function is used to calculate a non-trivial monetary value, or effect a hospital patient's treatment, nuclear power plant cooling, space shuttle trajectory, etc, then throwing an exception is pretty much mandatory.

I guess the real answer is that the "contract" needs to be extended to specify not just the expected arguments, but also what happens when the arguments are invalid.
Scorpio Send private email
Monday, October 27, 2008
 
 
If contract the specifies that the input is non-negative, then NEVER return any other value such as zero if the input is negative anyway. If you do that, you have effectively changed the contract: from now on the behaviour for negative inputs is defined and is negative inputs are valid and inside the contract.

Raising exceptions when contracts are broken, is in general, a very bad idea. Contract violations indicate programmer errors, not hardware flaws or user errors. User errors and hardware flaws should be designed for. Then exceptions are often the best idea: since the situation is 'exceptional' it is an easy trade-off between clear code and performance.

Contract violations are best not handled at all! Let the code explode in your face. Do not try to remedy the problems for a number of reasons:
- the further away from the location of the broken contract, the more difficult it is to make a proper analysis
- if your code is any good at all, violations should be very rare. Therefore, the exception handling code is seldom used ('zero or near-zero coverage') and a notorious hiding place for bugs. Speaking from experience: it is simpler to get the routines themselves correct then the exception handling code
- if the code explodes in your face, it is not possible at all to deny your quality problem. So the bug must be removed first. This is the best thing that can happen to your engineering.
- since the violation are caused by mistaken assumptions by the programmer the philosophical question arises how you would handle the violation at all. Some essential assumption by the developer was mistaken and you do not know which one or where (else it would have been inside the contracts terms), so how would you know what to do to repair the program to a correct known state.

See my blog for more information on design by contract and high-integrity programming: http://www.hello.nl .
Karel Thönissen Send private email
Monday, October 27, 2008
 
 
It should make clear, that you should *raise* the violation, but not *handle* it.

OP: assuming that the client will comply with the contract is totally unacceptable. Assumption is the mother of all f*ck-ups.
Karel Thönissen Send private email
Monday, October 27, 2008
 
 
The server should check compliance, not assume.
Karel Thönissen Send private email
Monday, October 27, 2008
 
 
What does "explode in your face" mean.

The service *must* do something. It might

    exit(0),
    core dump,
    tolerate the bad input and do something sensible
    tolerate, log a message and do something sensible
    not do useful work but still return "Success",
    insert a scrambled record into a database,
    return a failure code
    even throw an exception

in a server context where you have many clients relying on your service I think it's obvious that bringing down the server (exit or core dump) just because one client issued a bad request is "obviously" wrong.

I prefer to design to tolerate bad input, perhaps printing a message on the way. If I really can't do that then I think it's wholely misleading to silently ignore bad input and pretend that I did the work. I must report the error.

So, return a failure code or  throw an excption then the service has behaved responsibly.

Part of the game you are playing is to prepare your code for the future. Sure, today the client validates its input you don't apparently need to do so. In future some other guy writes a new client and slips up. They will be grateful that you put in the checks.

As for failure code v exception blocks. People forget to check return codes. People have to catch exceptions or let them percolate up either way they surface. If a programmer deliberately chooses to leave an exception handler empty
a). A code review can easily spot that
b). They chose to juggle chainsaws, there's only so much help a service provider can give.
Dave Artus Send private email
Monday, October 27, 2008
 
 
If it's a debug build or an alpha release it's helpful for it to blow up in an informative way. At least trigger a breakpoint.

If it's a release build it's not so clear cut whether the code should just bail. I'd err on the side of trying to muddle forward unless there's risk of permanent damage to data. But at any rate gather useful debugging info somewhere somehow.
Rowland
Monday, October 27, 2008
 
 
Dave, you mix up input validation and contract checking. These are two completely different things.

User input validation is checking whether the inputs from the user are corrects. User errors should never bring down the server, corrupt the data base or do anything else nasty. These are VALIDATIONS.

VIOLATIONS are when our very basic assumptions as a developer are wrong. These are checked with CONTRACTS. This is about developer assumptions like that memory will not run out, that indices are in bounds, that parameters related to each other in a certain invariant way, etc. If contracts are violated, then we always blame the developers, never the user. In principle it is impossible to handle violations, because it is logically impossible to get back to a known state from an unknown position (remember that the train of thought of the developer had been proven wrong!)

EXCEPTIONS are for exceptional, but not uncommon situations like failing hardware. These must be handled, and blame should not be placed on the user nor on the developer.

User input validation should only be done at the interfaces where data from unreliable sources enters the system, e.g. user input, non-trusted communication channels, etc. Once the data has been validated, it should be passed around freely.

Remember that in DbC every routine must specify its preconditions and postconditions. It is not allowed to defer contract checking to other routines. If routine #1 invokes a routine#2 where #2 fails to check the parameters with a precondition, and that cause a contract violation in some routine #3, then #2 is to blame, not #1. #2 should have checked the precondition. Maybe it then could have placed the blame on #1, but now it is in error, if only for not specifying and checking its preconditions. Remember that #1 could not be blamed in the initial situtation, since it could not be held against preconditions that where not specified in #2.

The same is true for validation. Input parameters for routines are checked against the preconditions. If these are violated, then we blame the developer of the client code, because we had been in the position to make sure that the parameter would have been correct: either be using contracts on its own parameters, or by validating the non-trusted inputs from the user.

In DbC, blame is not transitive.

I urge OP to read about design by contract and ignore some of the advice in this thread.
Karel Thönissen Send private email
Monday, October 27, 2008
 
 
"
VIOLATIONS are when our very basic assumptions as a developer are wrong. These are checked with CONTRACTS. This is about developer assumptions like that memory will not run out, that indices are in bounds, that parameters related to each other in a certain invariant way, etc. If contracts are violated, then we always blame the developers, never the user. In principle it is impossible to handle violations, because it is logically impossible to get back to a known state from an unknown position (remember that the train of thought of the developer had been proven wrong!)

EXCEPTIONS are for exceptional, but not uncommon situations like failing hardware. These must be handled, and blame should not be placed on the user nor on the developer.
"

While this sounds ok in theory, it just leads to hard to use/debug SOAs.

I also don't quite understand Karel's contention that letting the app "explode in your face" is different from throwing an exception. In both cases the ramifications are being felt by the "user" so why not throw a friendly exception explaining the problem vs. having it "explode in your face" with a cryptic error message. The end result is going to be the same, but the cryptic error is going to be much harder to diagnose.

You don't have to/want to handle all contract violations, but for the ones which can be easily anticipated, you should do so.

Monday, October 27, 2008
 
 
"If a programmer deliberately chooses to leave an exception handler empty
a). A code review can easily spot that"

Can't you say the same about error codes?
quant dev Send private email
Monday, October 27, 2008
 
 
Nope.
Almost H. Anonymous Send private email
Monday, October 27, 2008
 
 
Throwing exceptions let's a root level service catch it and fire off a bug report - such as the well known Windows dialog offering to send bug reports.

This is actually useful.

Just calling exit(-1) provides a fraction less information to the developer who might want to figure out how that error happened.

At a minimum, exit(-1) does not actually explain to the user why clicking on the menu made the program "go away". And by the time they're firing off an angry email they don't remember what the menu was and they won't be giving helpful stack trace information.

In the real world shit happens, and even if "the contract is broken" it's rarely useful to punish the user for your own bug. And it's rarely because the function was called with a hardcoded integer - instead some value was calculated and the test suite missed a path so you might want to know what happened.


Nah, sorry, exceptions are evil, wrong, and we should all call exit() to just terminate the application. That's much easier to code.

Monday, October 27, 2008
 
 
For object creation, whether eagerly via a constructor or lazily by setter injection, I always throw an exception if the contract is broken. The object should never be allowed to be in an invalid state.

For methods, it depends on the usage. Generally I throw an exception as the caller should know better and it indicates a flaw. If an error is not exceptional then I will use error codes or return a null value. It very much depends on the use-case, but I never expect users to be catching the exceptions I throw. I expect exceptions to indicate a real bug, not for use as control flow.

So, like many others here, I fall into the fail-fast, fail-hard spectrum.
Benjamin Manes Send private email
Monday, October 27, 2008
 
 
"If a programmer deliberately chooses to leave an exception handler empty
a). A code review can easily spot that"

Can't you say the same about error codes?


No, because in one case you are searching for the absence of something, which means that for each function call you need to know if it's void or returns somethinng to be checked for.

In the case of checked exceptions, if they are not caught then the compiler whinges, And if they are caught with an empty block: you have a searchable thing to look for

    { } and  { /* comment to fool the reviewer *? }

My eye more easily sees these than the *absence* of

    if ( rc < 0 ) ...

I choose to beleive that programmers want to do things right provided they understand and remember. I see exceptions as helping with the remember. You need to decide "oh I will leave this block empty" when the compiler makes you put it in, and I hope at that point the dligent concience kicks in.
Dave Artus Send private email
Tuesday, October 28, 2008
 
 
What I've seen happen with checked exceptions is that they are just added to "throws" clause in Java and somewhere at the top, there is a block saying "catch (Exception e) { print e.getMessage() to screen to confuse the user". Such propagation would be impossible with error codes, since it would require passing whole arrays around with aggregated exit codes.

I know that exit codes are cumbersome. What I would like to see is multiple return values, the second (third, ...) being the exit code, ignored at the caller's peril. I think that if someone ignores exit codes, they will not be handling exceptions correctly either. And with exit codes, at least it's harder to cheat (i.e. pass the "hot potato" around until it blows into user's face or is swallowed in some conveniently hidden place in the code).
quant dev Send private email
Tuesday, October 28, 2008
 
 
I would throw an InvalidParameter exception, or make the parameter unsigned int.

Tuesday, October 28, 2008
 
 
[quote]
OP: assuming that the client will comply with the contract is totally unacceptable. Assumption is the mother of all f*ck-ups.
[/quote]

Interesting, I always assumed I was the only one thinking that ;)
Marck
Wednesday, October 29, 2008
 
 
"Contract violations are best not handled at all! Let the code explode in your face."

This is well and good if the code will, in fact, "blow up in your face." More problematic is when the program keeps chugging along generating incorrect results. Wose still is if the incorrect results look correct to those on the outside.

"If contracts are violated, then we always blame the developers, never the user."

This is true. And as the developper I would like to know ASAP if I have violated a contract. That is why application needs to detect and handle violations of the contract in *some* way. Throw an exception. assert() the contractual obligation (causing the application to halt if a violation is discovered). Then if I do something I shouldn't have done I will find out immediately when I test it out during development. I don't have to wait until it goes into formal testing, or worse yet, into the field.
Anominal
Wednesday, October 29, 2008
 
 
If this method is only called by your internal code, assert and throw an InvalidParameter exception. If the method is only called by somebody else's code, then throw an InvalidParameter exception.

As for Karel's assertion that exceptions are only for "exceptional situations", many many people disagree with him. For example, Jeffrey Richter says:

"An exception is the violation of a programmatic interface's implicit assumptions."

In reality, if your component is being used by somebody else, it's that person's view of when s/he expects an exception that counts.
Mark Pearce Send private email
Monday, November 03, 2008
 
 
Well, yeah, but for example I'm writing a fairly general class (say, AdaptiveIntegrator) and expect it to be used by different groups in the company. They *do not* have common coding standards. It seems that the best option I have is to impose my views on them, then trying to conform to different standards.
quant dev Send private email
Monday, November 03, 2008
 
 
I follow the "principle of least surprise". But in the specific case mentioned by the OP, I would always throw an exception. The interface contract has been broken, and the caller needs to know that immediately.
Mark Pearce Send private email
Monday, November 03, 2008
 
 
Interesting discussion.

I've found that different cultures exist for different languages. The one I like most is Eiffel's. In Eiffel you can typically compile in two modes: release (no assertion is ever checked; it's as if you never wrote them) and diagnose (assertions are checked). Diagnose can be full (preconditions, postconditions and invariants) and basic (only preconditions).

In the first mode the behaviour is undefined. Don't call the function with wrong preconditions; just _don't_. You did? I told you not to do it. Nobody knows what will happen. By design, one thing is sure, though: the code will not check for your precondition in runtime. Only fortune will tell what happens.

The reasoning behind this is:

- Test your code. By all means, don't write code which calls a function without its preconditions. You had to read to see the function's name and parameters. Read, also, the preconditions, and fulfill them. If you don't know, put a check in the calling code.
- Design is clearer, because the function only does what it was designed for. It doesn't try to sort out every kind of mess.
- You'll have a correct and efficient release version.

The diagnose version is for your testing purposes.

And yes if you publish a library with a public API instead of an app, then in the input layer, _there_ you'll check the validity of your input. Not in all of your "inner" code.

However during my programming lifetime I'had to adapt to other (imho inferior) philosohpies, like Java's check everything, always. I don't like them. You end up messing legitimate exceptions (a database could not be opened, a file could not be found, a connection was lost in the middle of something) with all kinds of stupid runtime exceptions related to broken loosely written contracts.
Daniel_DL Send private email
Thursday, November 13, 2008
 
 
Follow-up. Bertrand Meyer in OOSC 2nd Edition ends up suggesting that you might want to leave precondition checks activated.

But then, your response in this case should be terminating, and clearly different from other exceptions. It should say "precondition violated" and terminate the program.

I still published an app without precondition checks because performance was critical. It was fun.
Daniel_DL Send private email
Thursday, November 13, 2008
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz