Code Duplication
By Adrian Sutton
Benjamin makes some good comments about code duplication and software design. I think by and large I agree with him though I do think that less code duplication implies better design, it’s just that there are also other factors which may be more important as well. The critical thing about eliminating duplicated code is to identify duplicated code based on it’s requirements, not on what it looks like. For instance, today I added a new method to our UIUtils class (GUIs are the most common place where duplicate code is created unnecessarily), the method took as parameters an OK button and a Cancel button, created a panel and put them in it in the appropriate order for the platform. It’s about 5 lines of code all up and it’s duplicated all over the place when it should not have been. In many cases, when this code is duplicated, it doesn’t actually look the same – it might use different layout managers, be mixed in with other code or a wide range of variations. However, the requirement is extremely clear cut: On Macintosh the cancel button is on the left and the ok on the right, on all other systems the ok is on the left and the cancel is on the right. The buttons are separated by a gap of 4 pixels. Regardless of where ok and cancel buttons are used or how they are implemented, that requirement must be upheld. Once I re-factored it out into a utility method it also became apparent that OK and Cancel buttons also have to have their preferred size set in a specific way – something we had another utility function for already since the behavior is similarly well defined. So a call to the sizing function is added to the layout function and we have a standard library function that gives us an ok and cancel button pair ready for adding to a dialog. There are other things that need to be set about ok and cancel buttons (for instance their action listeners are almost always identical because of the way we write our code) but that’s getting too big for a static utility method and it’s not always common to every ok and cancel button. Having said that, I really should re-factor out the cancel listener because I can’t think of a cancel button that has different requirements plus it has to be used for the escape key and a bunch of other places. Getting rid of the extra class files the anonymous listeners create would probably save us a hundred k or so. Coming back on topic, the design used isn’t correct in terms of object oriented programming, in fact it’s almost aspect oriented programming as it forms a cross cutting concern across all GUI dialogs, however it is extremely effective, has an extremely clearly defined purpose and is short and simple to understand even at a glance. Possibly the biggest indicator that a utility method should be used is that if the requirements for one dialog’s layout of the ok and cancel buttons changes – the requirements for every dialog will also change. The need for consistency in implementation is a big red flag that there should be a single implementation. So of course now if we ever support a new system that orders the ok and cancel buttons like Mac OS does or perhaps some other layout, we only have to make the change in one place. Essentially, I think Benjamin and I are saying the same thing in slightly different ways, except that I don’t class code that should behave different as duplicated code – only code that has the same requirements is duplicated. On a side note, Benjamin used the phrase:
bad design that comes about from prematurely reusing common-looking code Prematurely doing anything is always asking for trouble (including prematurely suggesting that something will cause trouble). It’s amazing how many people (myself included) still fall into the prematurely doing stuff trap either for reusing code, optimization, writing code, writing a design and the most common: prematurely shipping the product.