Thursday, 11 August 2011

Minimizing impact

While code-reviewing proposed changes to the HPCC-Platform project, I recently saw one that seemed questionable. I'm paraphrasing but the code that has been added was something like this:

#ifdef __APPLE__
some change that didn't seem particularly Apple-specific

I questioned the change - why was it needed and if so why was it only needed on Apple systems - and was told (in effect) "Technically, the change is correct for other platforms too, but it's not necessary because they don't check, and I wanted to minimize the impact".

There are times when it's appropriate to make a change in such a way that it "minimizes impact" at the expense of future maintainability and code readability - primarily in cases where an emergency patch is being made in a stable branch - but on an active code branch it's almost always a bad idea. You now have two versions of the code, one more correct than the other, and both needing to be maintained.

If you believe that a change you are proposing improves the code, then why would you want to minimize the amount of improvement? And if it doesn't improve the code, then you shouldn't be proposing it (and I won't be accepting it!)

No comments:

Post a Comment