|
This paper was originally written for the UK-BUG magazine.
In a dark, paranoid corner of my mind, I imagine the conversation went something like this:
"How many?"
"Two. We've got two Australians presenting at DCon 2002"
"How did this happen?"
"Well, they submitted papers, and the Advisory Board accepted them"
"Yes, but what are they like? Are they like that horrid Crocodile Hunter fellow, always saying 'Struth' and 'Crikey' and 'Dry as a dead dingo's donger'?"
"I don't think so. One of them even lives here, and the other one seems civilised enough on email"
"Well, I don't know. Do you think anyone will want to listen to them?"
"Maybe we should get them to write an article each for the User Group magazine. Then everyone can get a chance to check them out"
"OK.but if either one of them even mentions the Cricket, he's packing his bags"
OK, maybe this is just a symptom of some colonial inferiority complex, but this is what flashed through my mind when Joanna wrote asking me to submit a short article. Of course, this was quickly replaced by fear born of having no idea what to write about. So, in a move that I'm getting particularly good at, I promptly forgot about it, hoping that either inspiration would strike, or desperation would drive me to think up some topic of worth.
The Problem
Thankfully I didn't have long to wait. Whether it was desperation or inspiration, I'll leave for you to decide, but a few days later, I was doing a code review for a client. Usually when I perform code reviews, I try not to dictate rules to the development teams. It's bad enough that I'm an external consultant brought in to check what they are doing, but if I start laying down rules about what they are allowed to do and what they aren't, I'd have a mutiny on my hands. So usually, I find a way to get the team in question to come up with their own guidelines and rules, with a little gentle guidance from me. This way, they think they are breaking their own rules, rather than mine, and on the whole, are more inclined to stick to them.
Well, this case turned out to be an exception. With this particular team, we'd already been through this process many times before, and for the most part they were getting good at the whole code review process, and seeing benefits in terms of reduced bug rate. One of the guidelines they'd come up with was to test the pre-conditions and post-conditions in their methods. We'd decided that they should do this in all public and published methods. In addition, it probably wasn't a bad idea to do it in private and protected methods which were particularly hairy.
What I'm talking about here is basically having a series of tests at the start of each method which test the values of the input parameters, and any other variables used within the method. If your method expects that variable Foo will always be positive before the method executes, then test it, and if it happens not to be positive, raise an exception to that effect. Also, if after you've done all the business logic in your method, you expect variables etc to be in a particular state, then put some tests in to confirm those as well.
This is hardly revolutionary stuff. However, if done consistently, it can reduce your debugging time dramatically. And there was the problem. This development team were a smart enough bunch, just extremely busy. Quite often when under pressure from various people to get something done, they last thing they had time for was to include their pre and post-condition checks. That's OK, this is exactly why you have code reviews, to pick up these lapses.
When I spoke to the latest offender about the lack of any pre or post condition checks in his code, his response was basically "Yeah, I know, but it's a real pain in the bum to write all that stuff. Your business logic ends up mixed in with all these checks. Besides, our Unit Tests will pick any errors up". While I disagreed about the Unit Tests, which are there to test specific scenarios, and while very important, certainly aren't there after we ship, I had to admit he made some valid points. They were a pain to write, and they were very messy to look at.
So we had a choice. I could stamp my foot and tell him to go back and change his code or we could work around the problem. I usually try not to resort to foot stamping (unless we're talking about with statements, but that's another story), so I started thinking about how to get around his issues.
The Solution
The solution was quite simple really. If the checks are a pain to write, make them easier. If they are messy and distract from the business logic, put them somewhere else. I agree, this is stating the bleeding obvious, but often I find that good solutions come about by stating the obvious, then seeing how you can make it happen.
The first one was "make them easier to write". When we looked at the sorts of tests we we're doing, a lot of them fell into the same category. We were testing things like whether a parameter was less than, greater than or equal to some other value, between two other values, positive, negative, equal to zero, assigned, etc. Obviously different parameter types need different tests, but an awful lot of them were basically testing the same things.
The second goal was to "put them somewhere else". When we looked at their checks, a lot of the code written was not about the test, but was about what to do when the test failed. So there was a lot of code for constructing messages to display to the user and raising exceptions containing the constructed message.
Once we'd seen these things, the solution was standing out like a particular portion of canine anatomy. So, Listing 1 contains the definition of a class called TmtCheck. Note that it has a bunch of methods, like IsGreaterThan, IsBetween, IsAssigned. Also note that there are overloaded versions of each method, accepting different datatypes. This means that all the programmer has to think about is what test he wants to perform, and the compiler will find the correct method based on the datatypes he's testing.
Another thing you should note is that each method takes a bunch of string parameters like ValueDescr, MethodName, etc. These are used to build a meaningful Exception message to be displayed when the test fails, but as they are all defined with default values, they can be left out if desired. Lastly, all these methods are declared as class procedures, meaning that we don't need to create an instance of our TmtCheck object to be able to use it.
All this means that code like this:
If not ((Age < UpperAgeLimit) and (Age > LowerAgeLimit)) then
Raise Exception.Create('Age not within allowed range');
Is reduced to this:
TmtCheck.IsBetween(Age, LowerAgeLimit, UpperAgeLimit);
Considering that you might have 3 or 4 of these tests (or even more complex ones) at the beginning of a method, this is not a bad reduction in code. If it means that the test will actually get written, instead of pushed aside, then this also means a big reduction in debugging time should a value be passed in which is not valid. In addition, there are a couple of other benefits:
- All our tests now raise an exception of type EmtCheckFailed if they fail, rather than a motley collection of Exception types based on developer preference.
- If our tests do fail, and the developer has supplied values for the optional parameters in the various methods, it means we get a much nicer error message. In the example above, instead of getting a " Age not within allowed range " message, we could get a " TMyObject.Foo : Age (15) failed test : Must be between LowerAgeLimit (18) and UpperAgeLimit (75) ". I don't know about you, but I think the second version is much more helpful in tracking down the problem.
You might be wondering about the line at the bottom of Listing 1 , which says:
Check = TmtCheck;
Well, I pretty proudly presented this developer with my solution to his problem, and was he happy? Well, no actually. But at least his problem was minor. He thought that having to type the class name (ie. TmtCheck) before calling the methods was not only long (a whole 8 characters), but might be confusing to some of the newer guys who didn't know about class methods. So he wanted to call the class Check instead of TmtCheck.
Hmmm, a class name that doesn't start with T. Yeah, that'll be a lot less confusing. But rather than argue, and rather than introduce a function called Check that returned a class reference to TmtCheck, I opted to create another type declaration which maps to the original TmtCheck, to keep him happy. As it turns out, that's what the entire team is using, so it shows what I know.
You can probably guess what the implementation of each of our class methods looks like, so I won't waste space by providing a listing. I've centralised the building of the error message into a protected function, so each individual test method is pretty simple. We also didn't add all possible tests, just those that we could think of immediately, and figured the class would mature over time. When we found we needed a test that wasn't there, we'd add it. We did however, add a couple of generic tests called IsTrue and IsFalse, which simply take a Boolean expression, so they can be used kind of like the Assert procedure.
Why not Assert?
Which brings us to an important point. Why aren't we using Assert to do all this? Don't get me wrong, I'm a big fan of Assert, and one of the things I look for during a code review is liberal use of Assert, but it shouldn't be used everwhere. While it might be acceptable to use Assert for your post-conditions, I think it's madness to use them for pre-conditions.
Julian Bucknall expressed this very well in a recent Delphi Magazine article, but basically post-conditions are testing that your code worked as expected. Given sensible inputs, did my code generate sensible outputs. If someone compiles with Assertions turned off, you're not in too much trouble, provided your pre-conditions are still there, as they should catch any inputs that aren't sensible. However, if your pre-conditions were implemented with Asserts as well, then you could be in big trouble. If someone compiles with Assertions turned off, you've basically dropped your guard on all your methods. That's a big price to pay in the interests of a little performance boost.
Conclusion
Hopefully this will be useful to some of you. Since putting this together for one client, most of my other clients have started using it, and we're using it in our internal projects. I have to admit, even my own sourcecode contains more pre and post-condition checks now that they are easier to write. I also want to stress again that these are not a substitute for Unit Testing. Unit Testing and pre and post-conditions are aimed at solving different problems, and ideally you should use both. If I haven't put you off entirely, one of the sessions I'm presenting at DCon 2002 is on using DUnit to do Automated Unit Testing, so if this has piqued your interest, then there's a perfect opportunity to learn about it. (Ok, sorry for the blatant sales pitch).
Which leads directly into my last point. I'm not sure if this short article has served the purpose Joanna had in mind when asking me to write it, but if you are coming to DCon 2002, feel free to come and say Hi. I'm always interested in arguing the point with smart developers, and if we can do it over a beer or two, even better.
The latest version of the TmtCheck class can be found here . Feel free to download it and start using it. If you extend it with any tests that you think might be useful to others, let me know and I'll update the download.
About the Author
Malcolm Groves is Senior Consultant with Madrigal Technologies. He is a regular presenter at developer conferences in Australia, Europe and the United States, as well as an instructor for Delphi, C++Builder and JBuilder courses for Borland Australia. Malcolm also writes articles for developer magazines, including The Delphi Magazine. Prior to founding Madrigal, Malcolm was a Systems Engineer for Borland, as well as Delphi Product Manager for Australia and New Zealand. He lives in Sydney, Australia with his wife Julie and his dogs, Bonkers and Cookie
TmtCheck = class
protected
class procedure RaiseError(const ValueDescr, TestDescr,
MethodName : String); overload;
class function BuildDescr(const ValueDescr : string;
Value : Integer) : String; overload;
class function BuildDescr(const ValueDescr : string;
Value : Double) : String; overload;
class function BuildDescr(const ValueDescr : string;
const Value : String) : String; overload;
class function BuildMethodName(const MethodName : string): String;
public
// boolean tests
class procedure IsTrue(Value : Boolean;
const ValueDescr : String = '';
const MethodName : String = '');
class procedure IsFalse(Value : Boolean;
const ValueDescr : String = '';
const MethodName : String = '');
// Integer Tests
class procedure Equals(Value : Integer;
ComparisonValue : Integer;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure NotEquals(Value : Integer;
ComparisonValue : Integer;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsLessThan(Value : Integer;
ComparisonValue : Integer;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsLessThanOrEqualTo(Value : Integer;
ComparisonValue : Integer;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsGreaterThan(Value : Integer;
ComparisonValue : Integer;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsGreaterThanOrEqualTo(Value : Integer;
ComparisonValue : Integer;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsBetween(Value : Integer;
LowComparisonValue : Integer;
HighComparisonValue : Integer;
const ValueDescr : String = '';
const LowComparisonValueDescr : String = '';
const HighComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsBetweenOrEqualTo(Value : Integer;
LowComparisonValue : Integer;
HighComparisonValue : Integer;
const ValueDescr : String = '';
const LowComparisonValueDescr : String = '';
const HighComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsPositive(Value : Integer;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsNegative(Value : Integer;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsZero(Value : Integer;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsNotZero(Value : Integer;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
// double tests
class procedure Equals(Value : Double;
ComparisonValue : Double;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure NotEquals(Value : Double;
ComparisonValue : Double;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsLessThan(Value : Double;
ComparisonValue : Double;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsLessThanOrEqualTo(Value : Double;
ComparisonValue : Double;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsGreaterThan(Value : Double;
ComparisonValue : Double;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsGreaterThanOrEqualTo(Value : Double;
ComparisonValue : Double;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsBetween(Value : Double;
LowComparisonValue : Double;
HighComparisonValue : Double;
const ValueDescr : String = '';
const LowComparisonValueDescr : String = '';
const HighComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsBetweenOrEqualTo(Value : Double;
LowComparisonValue : Double;
HighComparisonValue : Double;
const ValueDescr : String = '';
const LowComparisonValueDescr : String = '';
const HighComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsPositive(Value : Double;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsNegative(Value : Double;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsZero(Value : Double;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsNotZero(Value : Double;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
// object tests
class procedure IsDescendantOf(Value : TObject;
ComparisonValue : TClass;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsNotDescendantOf(Value : TObject;
ComparisonValue : TClass;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsInstanceOf(Value : TObject;
ComparisonValue : TClass;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsNotInstanceOf(Value : TObject;
ComparisonValue : TClass;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsAssigned(Value : TObject;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsNotAssigned(Value : TObject;
const ValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsSameInstance(Value : TObject;
ComparisonValue : TObject;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
class procedure IsNotSameInstance(Value : TObject;
ComparisonValue : TObject;
const ValueDescr : String = '';
const ComparisonValueDescr : String = '';
const MethodName : String = ''); overload;
end;
Check = TmtCheck;
| 