Monday, 22 March 2010

Ronseal Rule

Now here is something you should think about every time you write a function. Does the function do what it says on the tin?

It's an important thing to think about. Some code is like this:

int CountThings()
// iterate through things
// while there, update their cache stuff
// and if one needs deleting, do it now
// return the count

That's pretty bad, but in a way, it's what a lot of systems do. Okay, so there are some optimisations to be made. Now, how about this one?

Thing *GetThing()
return mThingPtr;

That's not Ronseal either. And, it's prevalant in many systems I've worked on (including my own, oops).
No, I mean it. It's not Ronseal.
No, really. Have a look at what it's doing. It's not actually returning a Thing, it's returning a pointer to a thing...
You think I'm being picky? Well, most of the time I'd say you're right, but, what if I told you I could refactor it into two different functions?

Thing& GetThing()
return *mThingPtr;
bool ThingExists()
return mThingPtr != NULL;

Did you have an "ah-ha" moment there?

Okay, now think about this from the point of view of all those function calls you do to get objects, then check for NULL on return... You've had to infer two pieces of information from one call return value.
Now, go and fix your code.

No comments: