Firstly, let me say this post is just in regards to Delphi Windows apps, not other platforms.
Recently I was involved in a lively discussion on exceptions with my work mates. At least a few of them (if not all) felt that exceptions should not be used to alert the user to invalid user input and a message box should be used instead. For example:
procedure TEditForm.Save();
begin
if Length(txtName.Text) > 50 then
begin
ShowMessage('Name too long');
exit;
end;
// Save the record, etc
qrySave.Post;
end;
I believe every time you have a function (or procedure) that fails you should be raising an exception, even if it is in your form code. It would look like this:
procedure TEditForm.Save();
begin
if Length(txtName.Text) > 50 then
raise Exception.Create('Name too long');
// Save the record, etc
qrySave.Post;
end;
If you just show a message box and exit, then there's no way any code calling your Save method will know that it failed. For example let's say you have a procedure called SaveAndNew, which calls your Save method and then puts the form into a state ready for another new record:
procedure SaveAndNew();
begin
Save;
New;
end;
Without Save raising an exception, SaveAndNew will happily try to start a new a record without knowing whether Save succeeded or failed, making the message box approach actually quite hazardous. Okaaaay, that's not entirely true, there's a whole bunch of ways of letting your calling code know that your Save method failed without using exceptions, but I'm going to show you why all those ways suck monkey butt.
Alternative 1: Turn the Save method into a function which returns whether it saved or not.
Okay that can work in this simple example, but what if your Save method was already a function returning a useful value. If it was returning the name that got saved you could return an empty string to show it failed, or if it was returning the identity field as an integer you could perhaps return a negative number.
But what if the name is optional, then you can't rely on an empty string meaning it's failed or what if you return an integer and it's possible to have negative identity numbers that are perfectly valid? Sure you can return a special value meaning it failed using an unlikely result like "Sir Poopy Pants The Second" or -9999999999999 but I think we'd both agree that's really inelegant.
There are other alternatives similar to this like reserving the first letter of the string-based result to indicate success/failure or making the function return a record/class with one of the members being the result and another indicating success/failure, but these get even less elegant!
Alternative 2: Have an 'out/var Success: boolean' parameter
Which indicates whether the function worked or not. The downside here is that now you have to declare and pass an extra variable in your calling code everywhere you call your function. And there are bound to be times when you don't care to know whether your function worked or not and times that do care. But either way you have to declare that damn variable.
Also, what if you're adding validation to some existing function which previously didn't have any checks, you now have to change the parameters of that function to include your var: Success parameter and change the way it gets called everywhere that the function gets called. If you're calling the function 50 times in your application, then you've got 50 calls to change! No thanks!
Alternative 3: Have a global Success variable in your application/form.
Which gets set to true or false depending on whether your function worked or not.
TEditForm = class(TForm)
...
private
Success: boolean;
end;
...
procedure TEditForm.Save();
begin
Success := False;
if Length(txtName.Text) > 50 then
begin
ShowMessage('Name too long');
exit;
end;
// Save the record, etc
qrySave.Post;
Success := True;
end;
procedure SaveAndNew();
begin
Save();
if not Success then
exit;
New();
end;
I know some people who do this and while it works, the code feels disconnected/disjointed and still inelegant to me. Like, I call this method over here and then have to check this other variable over here to know whether I should continue on doing what I'm doing. On the other hand if the Save method raised an exception then we wouldn't have to check any variable in our SaveAndNew method to know whether we should call New or not as the exception will cause the program flow to jump right over our New code.
Alternative 4: Show your message box but then call Abort() afterwards
This is probably the best out of all the alternatives because Abort is at least raising an exception (an EAbort exception) causing all your calling code to stop running too, so SaveAndNew won't end up calling New - this is good! This is nearly as good as using your own exception in the first place except that if any calling code wants to know more about why it failed, it can handle the exception but it won't get any details out because it's just an empty EAbort exception and while you can create the EAbort error with a message yourself and manually raise it this method still has one flaw that all of the above alternatives share:
One bad thing about all these alternatives
Lets say you have some code that calls your Save method but doesn't want Save to show a message box if it failed! Then you'd have to have an extra flag somewhere, either as a parameter in the Save function or a global variable somewhere to say that you don't want Save to show a message box. Again that is unnecessary and inelegant compared to Save raising an exception. If Save raises an exception any calling code can decide to handle the exception preventing the message box appearing, or if they do want the message box to be shown they just don't handle the exception which would be the default. This is a consistent way of silencing validation messages without resorting to declaring extra flags and extra logic.
Okay, enough of these lame alternatives! But Exceptions also suck for their own reasons...right?
Okay, that's all the alternatives I care to explore here and if you can think of other alternatives that are not variations of the four above then before suggesting them I'd be asking if they are any more elegant and consistent than the ones above. Okay so the alternatives are not great, but Exceptions aren't perfect either are they?! Well, I think they just about are! But here are some perceived downsides to exceptions that I think deter people:
Perceived Downside 1: "But exceptions are meant to be exceptional"
Why would you say exceptions are meant to be exceptional? Can you back up your reasoning? Someone in the public eye/ear said this and it seems to be a phrase that just caught on. But was he talking about Delphi Windows Programs? I don't know but I doubt it. Maybe he was referring to dot net apps, maybe he was referring to web applications. I'm pretty sure I've heard someone briefly argue this point on DotNetRocks or HanselMinutes and he went on to say that all exceptions should be handled and that it should be really exceptional for an error to ever reach the user. Do you really agree with that? Do you see what implications are? Try imagining having to put try-except blocks around every UI event that you wrote and now tell me if you agree with that statement? How costly and counter-productive would that be for the programmer? And to what gain? A user will see a message box whether it's handled or not. And remember, when you write those exception handler blocks and show a message box, you're denying any other UI code that might be calling your UI code from knowing there was an error! Anyway, I still don't see why this argument is of any relevance to us Delphites - as all exceptions are in fact eventually handled so there's really no difference to the end user as you'll soon see.
Perceived Downside 2: Exceptions should be reserved for "genuine", unexpected errors, not validating user input (right tool for the right job)
Errr...why? Really, what's so different between a function in your UI code and a function in the VCL? How would you feel if TQuery.Execute didn't raise an exception when it failed but instead showed a message box to the user and returned false if it failed. Man that would suck! lol You wouldn't have any control over whether the user got the message or not and your program would have no idea what sort of error caused the problem.
So I can guess what you're thinking now - you might agree with me here when it comes to framework classes and functions but my form is at the top of the call stack - it's the "UI Layer" so why can't I simply use message boxes for validation errors there? Well because you never know when another bit of code will end up calling what you thought were "top level" functions in your form - suddenly they can become just like TQuery.Execute showing a message box except that perhaps you haven't got anything in place to say whether it failed or not. And the new code calling your code will happily continue on, oblivious to the fact that your Save method actually failed.
Perceived Downside 3: But I don't want the error message box to have a cross on it for measly validation errors
Okay, this a valid issue but fortunately it's easily solved. First of all, what I do in my code is have a custom exception class just for validation errors. It looks something like this:
EValidationError = class(Exception)
public
constructor Create(AControl: TWinControl;
AMessage: string);
end;
constructor EValidationError.Create(
AControl: TWinControl; AMessage: string);
begin
if CanFocus(AControl) then
AControl.SetFocus();
inherited Create(AMessage);
end;
As you can see, you can pass in the TWinControl that failed validation and it will set focus to it for you. So my Save code would look like this:
procedure TEditForm.Save();
begin
if Length(txtName.Text) > 50 then
raise EValidationError.Create(
txtName, 'Name too long');
// Save the record, etc
qrySave.Post;
end;
So how do you get rid of that cross?
Okay typically all exceptions in a Delphi Windows app actually do get handled whether you handle them or not. You read me. "Unhandled" in a Delphi Windows app really just means unhandled by your code. There's a loop somewhere in TApplication very high up in the call stack that checks for new windows messages and passes them onto our forms and controls, which in turn raise button click events and the like for us. Unhandled exceptions cause problem flow to flow back up our call stack right so all unhandled exceptions eventually hit this loop high up in the call stack where TApplication then handles the exception and shows the message box with the cross. But there's a way we can handle all unhandled exceptions just before the VCL shows this message box.
You drop a TApplicationEvents component onto your form/data module (one per application) and handle the OnException event, this event will get called every time an exception has gone unhandled by your code! It passes down the Exception object and a Handled boolean which you can set to True to stop the VCL from showing it's message box. So you can write code like the following to show validation errors without the cross:
procedure ApplicationEvents1Exception(
AException: Exception; var Handled: boolean);
begin
if AException is EValidationError then
begin
// Show a message box without the cross
ShowMessage(AException.Message);
Handled := True;
end
else
// For any other sort of error,
// let the VCL show handle it
Handled := False
end;
If you log all unhandled exceptions and don't want to log validation errors, then you can easily discriminate between validation errors and other errors using the same check as above.
Perceived Downside 4: But I don't want Delphi to pause my program every time an exception occurs that's just for some trivial validation error.
Fair enough. Fortunately you can tell Delphi which type of exceptions it shouldn't break on. It's found under Tools, Debugger Options. You'll notice that EAbort is in the list of exceptions that Delphi doesn't break on. Just add your EValidationError to the list and Delphi won't break on it.
To Sum Up
- You should not assume your "top level" UI methods won't get called by other code
- Simply showing a message box and then exiting
without providing any way for calling code to know that your function failed - is dangerous and can and will lead to buggy code!
- normally doesn't give the calling code any say in whether the message box should be shown or not!
- requires extra work to satisfy the 2 points above
- Raising exceptions for invalid user input
- is more elegant and often less code than it's alternatives
- is safer than it's alternatives
- can be tweaked with very little code to look just like a normal message box to the end user
- is consistent with what functions and classes in the VCL already do
- is a good practice for all the above reasons
Wow I'm impressed you made it this far...or did you just skip to the end? lol Well, I hope that if you didn't already agree with this logic (even without realising it) that I got you to at least consider converting your practices. :-)
....or at the very least that I never have to use your code because it'll be my code that eventually fails when yours doesn't raise an exception! lol jk ;-)
Posts
