Spot any errors? let me know, but Unleash your pedant politely please.

Sunday, 22 February 2009

Why readability and scope matter

I was reasonably lucky to have a decent lecturer in Polytechnic, who taught me some good programming practices. This was structured programming (Modula-2), rather than OOP, but they also apply to OOP. Essentially, it boiled down to two complementary goals: Low Coupling and High Cohesion. The goals are fairly simple to achieve by following a couple of simple rules:

1. Limit scope as much as possible.

2. A subroutine should do one thing only, and all the data it needs should be defined in its interface (its formal parameter list).

There are occasions to break the rules, such as when optimising. Breaking the rules for mere convenience, especially early in development is a definite no-no. I've done it, and it usually comes back to haunt me, and it usually needs refactoring to limit the scope.

It's arguably more difficult with OOP, because instance variables have global scope for an object, but not all methods should be allowed to alter them. It's probable that if you get into this situation, your object is too complex and should be split into smaller and simpler objects.

So that's scope.

Readability matters because code gets read far more often than it gets modified. Badly formatted code is difficult to read. Badly formatted code is confusing the reader.

I once spent several days trying to find a problem in a very long module. The module was too long, but the tools we were using made it difficult to split a lot of the code out. There was a pre-processor to flesh out system / messaging calls, but only against certain modules. The module at fault had grown to something like 10,000 lines of code. I knew he problem was in there, but I didn't have a reliable way to make it fail in unit testing - where I could debug. We didn't have great unit tests, and it wasn't possible to debug during system testing (when it failed)

The module was a bit of an unreadable mess. The indentation was all over the place and there were dozens of variables scoped to the highest level of the module.

I told my boss about this, and asked for permissions to take a few days to tidy up the indentation and fix the scoping issues. He told me we didn't have time. We were close to releasing, and we couldn't make those sort of changes.

I went with my gut. I lied about what I was doing in progress meetings and went ahead and fixed the indentation and refactored to limit the scope of everything as much as possible. I think I actually got rid of all the variables global to the module (There were the equivalent to instance variables, which remained, but these were defined elsewhere). It took about 3 days.

When I came to test again, with something more readable, as loosely coupled and highly cohesive as i could make it, I tried to find the bug again. I had gone.

I've actually no idea where the bug was, or what the change was that fixed it. The likely problem is that a global variable was being set when a local one should have been set, and this was now impossible with the new scoping restrictions. But I never found out exactly where.

I told my boss I'd found and fixed the bug, and gave some bullshit response to explain it.

No comments:

Post a Comment