Wednesday, February 9, 2011

Names are powerful things.

Read the list of colours, but try to call out the colour, not the word.

Red, Blue, GreenYellow, GreenRedBlueYellow

Names are powerful, they live in our head, and tell us things, we believe them, even against strong evidence.

When you read source code, it should mean something. Variable names and function names are the first window you have on their functionality.

int idx;

This already says a lot, but then someone goes and adds

int idx2;

If the first variable had been called sensorReadingsIndex and the second one was later added as controlOutputsIndex then when someone reads the code in 6 months time, the Confusion Index is reduced. I'm prepared to guess that sensorReadings[controlOutputsIndex]is probably wrong. Who can tell about sensorReadings[idx2] ?

Then you see code that looks like this, and you know what has happened.

If (processSensorReadings(...)) {
   // do stuff

When it was written, mostly likely it did not return anything, it just processed the sensor data, and someone added the return code. Of course they didn't change the name of the function. That would have involved changing all the calling code. (Yes, there are refactoring tools that do this)

What is processSensorData returning? You look into the function, and you see that it has lines of code like this

result = compareSensorReadingsWithPrevious(...);
return (result)

Eventually after we skip down 4 levels of code, we find the bug, someone returns true if the Reading is significantly different, or maybe true if the Reading is the same. Who can remember ?

Regardless of major refactoring and redesigning, names would have made life so much easier. Either names in the return code - using an enum instead of a bool or even simply changing things so that the result is obvious.

But you cannot simply call the function HasSensorReadingChanged() because now your code is misleading. This function actually processes the Reading in some way, and it happens to return some information. But someone looking at this code will now expect it to be a simple comparison, and won't expect it to "do processing"

bool sensorReadingHasChanged;
processSensorReading(..., out sensorReadingHasChanged)
if (sensorReadingHasChanged) {

And so on through the code....

Now, when I change the code for  I don't have to remember why I return true or false, the name says it all.

Yes, there are lots of ways to skin the cat, and there are lots of right ways, but there are also lots of things which you can do to avoid confusion. Names are powerful things. Choose them carefully, and unlike ships, there is no bad luck attached to renaming a badly named function. 

And note, this may not be the best way to do this, some people would argue it's not even a good way, that you should split up the function into two functions (and they may well be right) but this shows the thought process of trying to make things easier to understand, and harder to get wrong. 

No comments:

Post a Comment