Written By: Steve Zagieboylo, Senior Architect at Calavista
This is the second part in the series, “Technical Debt Patterns.”
The Abstraction Overlooked pattern is where you have some concrete class that has a “type” of some sort, and you find yourself basing some of its behavior on its type, either through if-then-else or switch statements. Often this occurs because the original programmer didn’t realize that there would be more than one type of this object, either because requirements have changed, he just didn’t think about it, or the entire project has grown larger than anyone ever imagined.
Example
Say you’re writing a tool for stock brokers to track the assets of their clients. One aspect of a client is his cash account, which your transactionally-sensitive code draws from on buys and adds to on sells. Your code includes a test on a withdrawal that you aren’t drawing more than is in the account, and rejects the transaction if it is. Your code works great, and the stockbroker is happy. Then he comes back with a new feature, Premium Customers. One aspect of a Premium Customer is that he is allowed to overdraw the cash account by some fixed amount, because we trust him. You edit your withdrawal code to check if this is a Premium Customer, and it now allows the transaction if it is within his overdraw limit. This is literally less than a line of code – just an extra test in the if statement. Then another type of customer is allowed to attach his bank savings account to his cash account, and you’re supposed to change the code to draw from that on an overdraw. And there will be more – you should have made the account an abstraction.
The Cost
The biggest aspect of the cost is not ongoing development, but ongoing quality and testing.
The cost for this pattern is a subtle one for many developers, because the biggest aspect of the cost is not ongoing development, but ongoing quality and testing. Every time there’s a new way a special type of account behaves, the developer is now editing code that is used by all accounts, with some possibility of introducing a bug to one of them. Sure, you have unit tests, and I’d even bet that, for this example, they’ve got enough coverage that you wouldn’t miss something. But can you say that when it is something less carefully tested than monetary transactions? As the cases get more complex and there are three or four places where there’s a behavior change based on the type, are you really sure you haven’t missed one?
The Fix
The fix for this problem is usually well-contained, and, if you do have those unit tests, fairly easy to confirm is correct.
1. Scope the fix first by declaring the class to be abstract and making a single concrete version of the class that has literally nothing in it. Then rebuild and see how many errors pop up. They will be all the places you’re calling the constructor of your now-abstract class. Ideally, your only errors will be in the one place that owns this object and in the unit tests which are specific to it. If there are a lot more, you might want to take a step back and ask yourself if the missing abstraction is a level higher (or at both levels). If you’re satisfied with the number of errors, make a factory to create the correct instance of the class, thinking about what concrete classes you’re going to end up with and what the factory will need to know to create the right one. (I’m fond of making the factory a static method of the abstract class or interface, but there are people who hate this approach and always want a separate class with this responsibility. I’m not quite willing to say they are wrong, but I’m also not going to change the way I do it. It’s a trade-off of clarity vs. simplicity, and I usually go for the latter.)
2. At this point you might decide you want an interface rather than an abstract class; that’s almost never a bad idea, even if there is a single abstract class that all the concrete classes extend. Extracting out the interface forces you really to think about what functionality is fundamental to the concept and what is implementation detail. I like making the abstract class, though, even with the interface. If you found yourself here, there’s probably a lot of functionality still that is common to all the concrete classes, and that code can stay in the abstract base. Let the highest level thing, the interface or the abstract base, keep the name by which the rest of the world knows this concept, and make the concrete classes have new names that describe what they are.
3. Then you have a pretty straightforward process of identifying what the concrete classes should be and moving the code out of the if-then-else or switch blocks into the appropriate location. Often you’ll find that the abstract methods which the concrete classes are overriding are protected – they are not the methods being exposed through the public interface, just a small part of them. There’s nothing wrong with this approach, but don’t overdo it, either. The abstract methods should make sense according to what they accomplish, not according to the details of how they get accomplished in the special cases you care about.
The abstract methods should make sense according to what they accomplish.
4. Once you’re a little way into the process, try to imagine the most extreme concrete class you might ever be expected to make. Ask yourself first whether or not you’ve passed all the information to the factory that you would need in order to know to create this. Then ask yourself if the abstract methods you’ve created would support the special cases that this class represents. Don’t add new abstract methods that exist only to support this imaginary case, but possibly rethink the ones you have, and make sure that the right information is available for them. It’s hard to quantify how, exactly, this experiment will inform the process, but I’ve almost never come out of it without some new bit of data.
5. The last step is to revisit your unit tests. If those tests were creating the original object directly, they need to change to call the factory. Take an inventory of all the unit tests to be sure all your concrete classes are being thoroughly tested, even all the methods that are not specialized in any way. You might want to create a new class AccountTester (for our example) that tests all the methods of any account, with parameters passed in for the expected results. Then your individual tests will consist of calling the factory, creating a helper with all the right parameters, then calling it. Think how happy you’ll be next year when you find yourself writing yet another concrete instance of this class.
