From ugly to pretty

I hold lectures in software engineering for over a decade now. One major topic is testing, specifically unit tests. Other corner stones are refactorings and code readability. So whenever I have the chance to challenge my students in cross-topic aspects of software development, it's almost always a source of insight for them and especially for me. But one golden moment holds a special place in my memory. This is the (rather elaborate, sorry) story of this moment.



During a lecture about unit tests with JUnit, my students had the task to develop tests for a bank account class. That's about as boring as testing can be - the account was related to a customer and had a current balance. The customer can withdraw money, but only some customers can overdraw their account. To spice things up a bit, we also added the mock object framework EasyMock to the mix. While I would recommend other mock frameworks for production usage, the learning curve of EasyMock is just about right for first time exposure in a "sheep dip" fashion.




Our first test dealt with drawing money from an empty account that can be overdrawn:



@Test public void canWithdrawOnCredit() { Customer customer = EasyMock.createMock(Customer.class); EasyMock.expect(customer.canOverdraw()).andReturn(true); EasyMock.replay(customer); Account account = new Account(customer); Euro required = new Euro(30); Euro cash = account.withdraw(required); assertEquals(new Euro(30), cash); assertEquals(new Euro(-30), account.balance()); EasyMock.verify(customer); }



The second test made sure that this withdrawal behaviour only works for customers with sufficient credit standing. We decided to pay out nothing (0 Euro) if the customer tries to withdraw more money than his account currently holds:



@Test public void cannotTakeUpCredit() { Customer customer = EasyMock.createMock(Customer.class); EasyMock.expect(customer.canOverdraw()).andReturn(false); EasyMock.replay(customer); Account account = new Account(customer); Euro required = new Euro(30); Euro cash = account.withdraw(required); assertEquals(Euro.ZERO, cash); assertEquals(Euro.ZERO, account.balance()); EasyMock.verify(customer); }



As you can tell, a lot of copy and paste was going on in the creation of this test. Just look at the name of the local variable "required" - it's misleading now. Right up to this point, my main topic was the usage of the mock framework, not perfect code. So I explained the five stages of normalized mock-based unit tests (initialize, train mocks, execute tested code, assert results, verify mocks) and then changed the topic by expressing my displeasure about the duplication and the inferior readability of the code (it even tries to trick you with the "required" variable!). Now it was up to my students to improve our situation (this trick works only a few times for every course before they preventively become even pickier than me). A student accepted the challenge and gave advice:



FIRST STEP: EXTRACT METHOD REFACTORING



The obvious first step was to extract the duplication in its own method and adjust the calls by their parameters. This is an easy refactoring that will almost always improve the situation. Let's see where it got us. Here is the extracted method:



protected void performWithdrawalTestWith( boolean customerCanOverdraw, Euro amountOfWithdrawal, Euro expectedCash, Euro expectedBalance) { Customer customer = EasyMock.createMock(Customer.class); EasyMock.expect(customer.canOverdraw()).andReturn(customerCanOverdraw); EasyMock.replay(customer); Account account = new Account(customer); Euro cash = account.withdraw(amountOfWithdrawal); assertEquals(expectedCash, cash); assertEquals(expectedBalance, customer.balance()); EasyMock.verify(customer); }



And the two tests, now really concise:



@Test public void canWithdrawOnCredit() { performWithdrawalTestWith( true, new Euro(30), new Euro(30), new Euro(-30)); }



@Test public void cannotTakeUpCredit() { performWithdrawalTestWith( false, new Euro(30), Euro.ZERO, Euro.ZERO); }



Well, that did resolve the duplication indeed. But the test methods now lacked any readability. They appeared as if somebody had extracted all the semantics out of the code. We were unhappy, but decided to interpret the current code as an intermediate step to the second refactoring:



SECOND STEP: INTRODUCE EXPLAINING VARIABLE REFACTORING



In the second step, the task was to re-introduce the semantics back into the test methods. All parameters were nameless, so that was our angle of attack. By introducing local variables, we gave the parameters meaning again:



@Test public void canWithdrawOnCredit() { boolean canOverdraw = true; Euro amountOfWithdrawal = new Euro(30); Euro payout = new Euro(30); Euro resultingBalance = new Euro(-30); performWithdrawalTestWith( canOverdraw, amountOfWithdrawal, payout, resultingBalance); }



@Test public void cannotTakeUpCredit() { boolean canOverdraw = false; Euro amountOfWithdrawal = new Euro(30); Euro payout = Euro.ZERO; Euro resultingBalance = Euro.ZERO; performWithdrawalTestWith( canOverdraw, amountOfWithdrawal, payout, resultingBalance); }



That brought back the meaning to the test methods, but didn't improve readability. The code wasn't intentionally cryptic any more, but still far from being intuitively understandable - and that's what really readable code should be. If even novices can read your code fluently and grasp the main concepts in the first pass, you've created expert code. I challenged the student to further transform the code, without any idea how to carry on myself. My student hesitated, but came up with the decisive refactoring within seconds:



THIRD STEP: RENAME VARIABLE REFACTORING



The third step doesn't change the structure of the code, but its approachability. Instead of naming the local variables after their usage in the extracted method, we name them after their purpose in the test method. A first time reader won't know about the extracted method (and preferably shouldn't need to know), so it's not in the best interest of the reader to foreshadow its details. Instead, we concentrate about telling the reader a coherent story:



@Test public void canWithdrawOnCredit() { boolean aCustomerThatCanOverdraw = true; Euro heWithdraws30Euro = new Euro(30); Euro receivesTheFullAmount = new Euro(30); Euro andIsNow30EuroInTheRed = new Euro(-30); performWithdrawalTestWith( aCustomerThatCanOverdraw, heWithdraws30Euro, receivesTheFullAmount, andIsNow30EuroInTheRed); }



@Test public void cannotTakeUpCredit() { boolean aCustomerThatCannotOverdraw = false; Euro heTriesToWithdraw30Euro = new Euro(30); Euro butReceivesNothing = Euro.ZERO; Euro andStillHasABalanceOfZero = Euro.ZERO; performWithdrawalTestWith( aCustomerThatCannotOverdraw, heTriesToWithdraw30Euro, butReceivesNothing, andStillHasABalanceOfZero); }



If the reader is able to ignore some crude verbalization and special characters, he can read the test out loud and instantly grasp its meaning. The first lines of every test method are a bit confusing, but necessary given .



The result might remind you a lot of and that's probably not by chance. In a few minutes during that programming exercise, my students taught themselves to think in scenarios or stories when approaching unit tests. I couldn't have taught it any better - instead, I got enlightened by this exercise, too.
Full Post