Refactoring Teaser IV Solution Summary Report
Sunday, September 27th, 2009| Topic | Before | After |
|---|---|---|
| Project Size | Production Code
Test Code
|
Production Code
Test Code
|
| Code Coverage |
|
|
| Cyclomatic Complexity |  |
 |
| Coding Convention Violation | 85 | 0 |
Archive for the ‘Testing’ Category
Embracing Context Objects with Fluent Interfaces for my Tests
Thursday, September 24th, 2009Of late I’ve been toying around with a new way of using Fluent Interfaces with a Context Object for my Tests. Esp. when I’m using Mockito.
In this post (Fluent Interfaces improve readability of my Tests), I’ve taken an example and demonstrated how I’ve evolved my tests to be more expressive. In my quest for getting my tests to communicate precisely to-the-point by hiding everything else which is noise, I’ve stared exploring another way of using Fluent Interfaces.
Following is an example:
1 2 3 4 5 6 7 8 9 | @Test public void redirectSubDomainsPermanently() { lets.assume("google.com").getsRedirectedTo("google.in").withSubDomain(); response = domainForwardingServer.process(requestFor("blog.google.com")); lets.assertThat(response).contains(StatusCode.PermanentRedirect) .location("google.in/blog").protocol("HTTP/1.1") .connectionStatus("close").contentType("text/html") .serverName("Directi Server 2.0"); } |
lets and on are both Context objects which provide fluent, domain specific api to make the test very easy to read (communicative and expressive). It also helps me hide all my mocking/stubbing related code.
If you compare this with the original code, you can get a sense of what I’m talking about:
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 | @Test public void redirectSubDomainsPermanently() { when(request.hostName()).thenReturn("blog.google.com"); when(request.protocol()).thenReturn("HTTP/1.1"); when(request.path()).thenReturn("/"); domain.setDomain("blog.google.com"); domain.subDomainForwarding(true); domain.setForward("google.in"); response = domainForwardingServer.processMessage(request); assertStatus(StatusCode.PermanentRedirect); assertLocation("google.in/blog"); assertProtocol("HTTP/1.1"); assertConnectionStatusIs("close"); assertContentType("text/html"); assertServerName("Directi Server 2.0"); } |
Another example showing the Context object and Fluent Interface style is
1 2 3 4 5 6 | @Test public void avoidRestrictedWordsInIds() { lets.assume("naresh").isARestrictedUserName(); List<String> suggestions = suggester.optionsFor(naresh_from_mumbai); lets.assertThat(suggestions).are("[email protected]", "[email protected]", "[email protected]", "[email protected]"); } |
As I said, I’m still toying around with this idea. If this works well, may be it will be part of some mocking framework soon.
Everything else is just Noise
Tuesday, September 22nd, 2009Recently I was working on some code. The code was trying to tell me many things, but I was not sure if I was understanding what it was trying to communicate. It just felt irrelevant or noise at that moment. Somehow the right level of abstraction was missing.
When I started I had:
1 2 3 4 5 6 7 8 9 10 | private final UserService userService = createMock(UserService.class); private final DomainNameService dns = createMock(DomainNameService.class); private final RandomNumberGenerator randomNumberGenerator = new RandomNumberGenerator() { @Override public long next() { return 9876543210L; } }; private final IdentityGenerator identityGenerator = new IdentityGenerator(randomNumberGenerator, dns, userService); private final User naresh_from_mumbai = new User("naresh", "jain", "mumbai", "india", "indian"); |
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 | @Test public void avoidRestrictedWordsInIds() { expect(dns.isCelebrityName("naresh", "jain")).andStubReturn(false); expect(dns.validateFirstPartAndReturnRestrictedWordIfAny("naresh")).andStubReturn("naresh"); expect(dns.isCelebrityName("nares", "jain")).andStubReturn(false); expect(dns.validateFirstPartAndReturnRestrictedWordIfAny("nares")).andStubReturn(null); expect(dns.validateSecondPartAndReturnRestrictedWordIfAny("jain")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(dns.isCelebrityName("nares", "india")).andStubReturn(false); expect(dns.isCelebrityName("naresh", "india")).andStubReturn(false); expect(dns.validateFirstPartAndReturnRestrictedWordIfAny("nares")).andStubReturn(null); expect(dns.validateSecondPartAndReturnRestrictedWordIfAny("india")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(dns.isCelebrityName("nares", "indian")).andStubReturn(false); expect(dns.isCelebrityName("naresh", "indian")).andStubReturn(false); expect(dns.validateFirstPartAndReturnRestrictedWordIfAny("nares")).andStubReturn(null); expect(dns.validateSecondPartAndReturnRestrictedWordIfAny("indian")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(dns.isCelebrityName("nares", "mumbai")).andStubReturn(false); expect(dns.isCelebrityName("naresh", "mumbai")).andStubReturn(false); expect(dns.validateFirstPartAndReturnRestrictedWordIfAny("nares")).andStubReturn(null); expect(dns.validateSecondPartAndReturnRestrictedWordIfAny("mumbai")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); replay(userService, dns); List<String> generatedIDs = identityGenerator.getGeneratedIDs(naresh_from_mumbai); List<String> expectedIds = ids("[email protected]", "[email protected]", "[email protected]", "[email protected]"); assertEquals(expectedIds, generatedIDs); verify(userService, dns); } |
As you can see, my first reaction after looking at this code was that there is too much going on, most of which is duplicate. So cleaned it up a bit and made it more expressive by
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 | private final Context lets = new Context(userService, dns); @Test public void avoidRestrictedWordsInIds() { lets.assume("naresh").plus("jain").isNotACelebrityName(); lets.assume("naresh").isARestrictedUserName(); lets.assume("nares").plus("jain").isNotACelebrityName(); lets.assume("nares").isNotARestrictedUserName(); lets.assume("jain").isNotARestrictedDomainName(); lets.assume().identity("[email protected]").isAvailable(); lets.assume("nares").plus("india").isNotACelebrityName(); lets.assume("nares").isNotARestrictedUserName(); lets.assume("india").isNotARestrictedDomainName(); lets.assume().identity("[email protected]").isAvailable(); lets.assume("nares").plus("indian").isNotACelebrityName(); lets.assume("nares").isNotARestrictedUserName(); lets.assume("indian").isNotARestrictedDomainName(); lets.assume().identity("[email protected]").isAvailable(); lets.assume("nares").plus("mumbai").isNotACelebrityName(); lets.assume("nares").isNotARestrictedUserName(); lets.assume("mumbai").isNotARestrictedDomainName(); lets.assume().identity("[email protected]").isAvailable(); List<String> generatedIds = suggester.generateIdsFor(naresh_from_mumbai); lets.assertThat(generatedIds).are("[email protected]", "[email protected]", "[email protected]", "[email protected]"); } |
By introducing a new class called Context and moving all the mocking code into that, my test looked lot more clear. I was also able to create an abstraction that could communicate intent much more easily.
Next I reduced the clutter further by creating another level of abstraction as follows
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 | @Test public void avoidRestrictedWordsInIds() { lets.assume("naresh", "jain").isNotACelebrityName(); lets.assume("naresh").isARestrictedUserName(); for (final String[] identityTokens : list(_("nares", "jain"), _("nares", "india"), _("nares", "indian"), _("nares", "mumbai"))) { lets.assume(identityTokens[0], identityTokens[1]).isNotACelebrityName(); lets.assume(identityTokens[0]).isNotARestrictedUserName(); lets.assume(identityTokens[1]).isNotARestrictedDomainName(); lets.assume().identity(identityTokens[0] + "@" + identityTokens[1] + ".com").isAvailable(); } List<String> generatedIds = suggester.generateIdsFor(naresh_from_mumbai); lets.assertThat(generatedIds).are("[email protected]", "[email protected]", "[email protected]", "[email protected]"); } |
But at this point, even though the code ended up being very dense, it was very difficult to understand what was going on and why so. In a desperate search for simplicity and better communication, I ended up with
1 2 3 4 5 6 | @Test public void avoidRestrictedWordsInIds() { lets.assume("naresh").isARestrictedUserName(); List<String> generatedIds = suggester.suggestIdsFor(naresh_from_mumbai); lets.assertThat(generatedIds).are("[email protected]", "[email protected]", "[email protected]", "[email protected]"); } |
What is interesting about this is that I made some simple assumption saying:
- every name is not a celebrity name unless specified
- every user name is a valid (non-restricted) user name unless specified
- every domain name is a valid (non-restricted) domain name unless specified
- every identity is available unless specified
All these assumptions are now capture in my Context object and rest of my tests can happily focus on what really matters. I really liked the way this reduced the clutter in my tests without compromising on communication.
Confronting the Fear of Legacy Code
Wednesday, September 16th, 2009When faced with Legacy Code, I’ve found 3 possible options to deal with them:
- Leave it alone for now: Very rarely used, code seems to work fine.
- Piecemeal Refactoring: When its difficult to understand what the code does and how it does what it does. Its time for safe, slow and cumbersome refactoring process.
- Rewrite: When its clear what the code does, but it very difficult to understand how it does what it does, it time to rescue the code by rewriting it from scratch. This can be applied at various levels (whole code base, single module, class or method).
To Rewrite or to Refactor?
One can easily spend hours or days trying to refactor some code, when clearly (in retrospect) rewriting the code would be a better option. Sometimes you decide its better to rewrite the code and end up implementing something that does not work in all situations or we miss out something important. Unfortunately there is no clear guideline when I would choose to refactor code v/s rewrite the code. The key to me is, if I understand what the code does not necessarily how it does what it does, then its time to rewrite the code.
Rewriting code: Play it safe
The analogy I use is, rewriting code is like building bridges. You know that the bridge helps you get from point A to point B. It might be very complicated and risky to use the bridge any more. But that does not mean you’ll go and blow the bridge apart. Instead you would slowing start building a new bridge along side. When the new bridge is ready, you would divert a sample traffic on this bridge and see if it actually works. If it does, then you migrate all the traffic to the new bridge and blow the old bridge apart.
I use the very same technique when rewriting code. During the process, I might leave the code working but in a much more messier (worse) state. During CodeChef TechTalks in Bangalore, Sai told me that he refers to this as an “Expand and Contract” cycle. You are temporarily expanding your code base so that you can come back and clean it up.
When I’m rewriting code, I find black-box style automated tests very helpful. If you don’t have tests, it might be worth investing the time to write a few.
Where to begin Refactoring Code
- Outside-In: Start from a higher-level and refactor (delve) into the crux
- Inside-Out: Start refactoring the crux and work your way out
At times its difficult to identify the crux and I spend some time exploring (via refactoring) before I can choose an approach. Tests can be a great probe to understand the code.
When refactoring legacy code, I usually use the Scaffolding Technique to break the Catch 22 situation (To refactor we need tests, to write tests we need to refactor). Scaffolding tests don’t necessarily have to be UI tests, I’ve used Unit tests as scaffolding tests as well.The key thing is they are temporary and meant to help you get started.
Thanks to the folks @ the Legacy Code BoF @ CodeChef TechTalks in Bangalore who prompted me to write this blog.
Redefining Legacy Code
Wednesday, September 16th, 2009Michael Feathers did a great job by redefining legacy code to: “Code without Tests”.
Over the years, I’ve dealt with code which had tests (unit, functional or both). Some of it was even test driven. But it was extremely difficult to understand and maintain the code. The code-base exhibited the same problems as Legacy code.
What does this mean? IMHO, it means we need to broaden our definition of Legacy Code.
Legacy code is code that developers fear facing. Legacy code does not communicate its intent and has a very convoluted design. It is code with high viscosity which encourages sloppy job by the developers and makes it extremely difficult for them to do the right thing. Abundance of Code Smells, lack of Tests, long feedback cycles, unpredictability, etc : all of these are contributing factors.
Value of End to End Functional Checks
Tuesday, September 8th, 2009At the Agile 2009 conference I attended a workshop by Alro Belshee and Jim Shores on “Slow and Brittle: Replacing End to End Tests“. In this workshop they presented a couple of cases where End-to-End checking (testing) became a huge issues as the tests became slow, brittle and expensive. While I agree with them and have seen cases of checks (not just end-to-end checks) becoming a maintenance nightmare. Many times, checks create an illusion of safety, but really don’t add much value.
While some of it can be attributed to the poor scope/focus selection for checking (end-to-end or unit), I also think a lot has to do with poor skills, lack of understanding and rigid/flawed organizational structures.
During the workshop, the group I participated in, came up with following points as reasons why people write end-to-end checks:
- Document the interaction and intent
- Being Inclusive: Testers and Programmers can collaborate
- Scaffolding
- Good starting point for dealing with legacy code
- Early win: Easy to show people how to automate checking and get them started
- Good starting point to learn/explore more about the system
- Freedom and Confidence to Refactor code
- Ease: (sometimes they are very easy to create)
- Highlight Design Problems: If you see duplication in your tests, sometimes it could mean poor design.
I know other teams had several other interesting points. So the question is what other approach can you use to do away with end-to-end checking?
One of my proposals was to look more serious at “Eating your own dog food“. I also very much digg the idea about “Self-learning and Self-testing systems”.
Checking v/s Testing
Tuesday, September 8th, 2009Testing is explorative, probing and learning oriented.
Checking is confirmative (verification and validation of what we already know). The outcome of a check is simply a pass or fail result; the outcome doesn’t require human interpretation. Hence checking should be the first target for automation.
James Bach points out that checking does require some element of testing; to create a check requires an act of test design, and to act upon the result of a check requires test result interpretation and learning. But its important to distinguish between the two because when people refer to Testing they really mean Checking.
Why is this distinction important?
Michael Bolton explains it really well. He says:
A development strategy that emphasizes checking at the expense of testing is one in which we’ll emphasize confirmation of existing knowledge over discovery of new knowledge. That might be okay. A few checks might constitute sufficient testing for some purpose; no testing and no checking at all might even be sufficient for some purposes. But the more we emphasize checking over testing, and the less testing we do generally, the more we leave ourselves vulnerable to the Black Swan.
Refactoring Teaser IV – Step 1
Wednesday, August 12th, 2009So far, most of the refactoring teasers we’ve looked at, have suffered because of lack of modularity and with primitive obsession. This refactoring teaser is quite the opposite. Overall the code base is decent sized. So instead of trying to solve the whole problem in one go, let’s take it one step at a time.
Download the Source Code here: Java or C#.
In the first step, I want you to focus on the PartialAcceptanceTest.
Test Setup:
private final Country country = new Country("IN", "India", "Indian"); private final LocationInformation location = new LocationInformation(country, "Mumbai"); private final UserService userService = createMock(UserService.class); private final DomainNameService domainNameService = createMock(DomainNameService.class); private final SuggesntionsUtil utils = new SuggesntionsUtil(domainNameService); private final RandomNumberGenerator randomNumberGenerator = new RandomNumberGenerator() { @Override public String next() { return "_random"; } }; private final ChildSuggestionFactory childSuggestionFactory = new ChildSuggestionFactory(userService, utils, randomNumberGenerator); private final SuggestionStrategyFactory suggestionsFactory = new SuggestionStrategyFactory(childSuggestionFactory); private final IdentityGenerator identityGenerator = new IdentityGenerator(suggestionsFactory); |
First Test (Happy Path)
@Test public void generateIdsUsingNameLocationAndNationality() { expect(domainNameService.isCelebrityName("Naresh", "Jain")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Jain")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "India")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("India")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "Indian")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Indian")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "Mumbai")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Mumbai")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); replay(userService, domainNameService); SuggestionParam suggestionParam = new SuggestionParam(location, "Naresh", "Jain"); List generatedIDs = identityGenerator.getGeneratedIDs(suggestionParam); List expectedIds = ids("[email protected]", "[email protected]", "[email protected]", "[email protected]"); assertEquals(expectedIds, generatedIDs); verify(userService, domainNameService); } |
Second Test
@Test public void avoidRestrictedWordsInIds() { expect(domainNameService.isCelebrityName("Naresh", "Jain")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn("Naresh"); expect(domainNameService.isCelebrityName("Nares", "Jain")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Nares")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Jain")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Nares", "India")).andStubReturn(false); expect(domainNameService.isCelebrityName("Naresh", "India")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Nares")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("India")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Nares", "Indian")).andStubReturn(false); expect(domainNameService.isCelebrityName("Naresh", "Indian")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Nares")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Indian")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Nares", "Mumbai")).andStubReturn(false); expect(domainNameService.isCelebrityName("Naresh", "Mumbai")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Nares")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Mumbai")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); replay(userService, domainNameService); SuggestionParam suggestionParam = new SuggestionParam(location, "Naresh", "Jain"); List generatedIDs = identityGenerator.getGeneratedIDs(suggestionParam); List expectedIds = ids("[email protected]", "[email protected]", "[email protected]", "[email protected]"); assertEquals(expectedIds, generatedIDs); verify(userService, domainNameService); } |
@Test public void avoidCelebrityNamesInGeneratedIds() { expect(domainNameService.isCelebrityName("Naresh", "Jain")).andStubReturn(true); expect(domainNameService.isCelebrityName("Nares", "Jai")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Nares")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Jai")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "India")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("India")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "Indian")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Indian")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "Mumbai")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Mumbai")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); replay(userService, domainNameService); SuggestionParam suggestionParam = new SuggestionParam(location, "Naresh", "Jain"); List generatedIDs = identityGenerator.getGeneratedIDs(suggestionParam); List expectedIds = ids("[email protected]", "[email protected]", "[email protected]", "[email protected]"); assertEquals(expectedIds, generatedIDs); verify(userService, domainNameService); } |
@Test public void appendCurrentYearWithFirstNameIfIdIsNotAvailable() { expect(domainNameService.isCelebrityName("Naresh", "Jain")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Jain")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(false); expect(domainNameService.isCelebrityName("Naresh2009", "Jain")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh2009")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Jain")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "India")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("India")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "Indian")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Indian")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "Mumbai")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Mumbai")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); replay(userService, domainNameService); SuggestionParam suggestionParam = new SuggestionParam(location, "Naresh", "Jain"); List generatedIDs = identityGenerator.getGeneratedIDs(suggestionParam); List expectedIds = ids("[email protected]", "[email protected]", "[email protected]", "[email protected]"); assertEquals(expectedIds, generatedIDs); verify(userService, domainNameService); } |
@Test public void appendRandomNumberWithFirstNameIfIdIsNotAvailable() { expect(domainNameService.isCelebrityName("Naresh", "Jain")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Jain")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(false); expect(domainNameService.isCelebrityName("Naresh2009", "Jain")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh2009")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Jain")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(false); expect(domainNameService.isCelebrityName("Naresh_random", "Jain")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh_random")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Jain")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "India")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("India")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "Indian")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Indian")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); expect(domainNameService.isCelebrityName("Naresh", "Mumbai")).andStubReturn(false); expect(domainNameService.validateFirstPartAndReturnRestrictedWordIfAny("Naresh")).andStubReturn(null); expect(domainNameService.validateSecondPartAndReturnRestrictedWordIfAny("Mumbai")).andStubReturn(null); expect(userService.isIdentityAvailable("[email protected]")).andStubReturn(true); replay(userService, domainNameService); SuggestionParam suggestionParam = new SuggestionParam(location, "Naresh", "Jain"); List generatedIDs = identityGenerator.getGeneratedIDs(suggestionParam); List expectedIds = ids("[email protected]", "[email protected]", "[email protected]", "[email protected]"); assertEquals(expectedIds, generatedIDs); verify(userService, domainNameService); } |
Some helper method:
private List ids(final String... ids) { return Arrays.asList(ids); } |
Signs of a Healthy Codebase
Tuesday, August 11th, 2009I’ve become a big fan of displaying metric using Treemaps. Julias Shaw‘s Panpoticode is a great tool to produce useful design metric in the treemap format for your Java project.
In the past, I’ve used these graphs to show Before and After snapshots of various projects after a small refactoring effort. In this blog I want to show you a healthy project’s codebase and highlight somethings that makes me feel comfortable about the codebase. (Actually there is not much to talk, a picture is worth a thousand words.)
Following is the code coverage report from a project:
Couple of quick observations:
- Majority of the code has coverage over 75% (Our goal is not to have every single class with 100% code coverage. Code Coverage does not talk about Quality of your tests.)
- There is a decent distribution of code across packages, classes and methods. (No large boxes standing out.)
- You don’t see large black patches (ones you see are classes that were mocked out for testing).
Lets look at the complexity graph:
- Except for a couple of methods, most of them have Cyclomatic Complexity under 5.
- You don’t see large red or black boxes which are clear indicators of complex code.
Panopticode combined with CheckStyle, FindBugs and JDepends can give you a lot more info to check the real pulse of your codebase.
Refactoring Teaser III
Wednesday, August 5th, 2009This time a simple one.
Following test will help you understand the code:
public class MyLoggerTest implements Console { private String msg; private MyLogger logger = new MyLogger(this); @Test public void handleNonIOExceptions() { logger.error(new IllegalArgumentException("Ignore Exception")); assertEquals("SEVERE: Dying due to exception : Ignore Exception", msg); } @Test public void ignoreSpecificIOExceptions() { String errorMsg = "Broken pipe:" + Math.random(); logger.error(new IOException(errorMsg)); assertEquals("FINE: Ignoring Exception for : " + errorMsg, msg); } @Test public void handleGenericIOExceptions() { String errorMsg = "Random IO Error:" + Math.random(); logger.error(new IOException(errorMsg)); assertEquals("SEVERE: Dying due to exception : " + errorMsg, msg); } @Override public void write(final String msg) { this.msg = msg; } } |
public class MyLogger { private final Console out; public MyLogger(final Console out) { this.out = out; } private static final String[] IGNORED_IOEXCEPTION_MESSAGES = { "An existing connection was forcibly closed by the remote host", "Connection reset by peer", "Broken pipe", "Connection timed out", "No route to host", }; public void error(final Throwable t) { if (isIgnored(t)) { out.write("FINE: Ignoring Exception for : " + t.getMessage()); } else { out.write("SEVERE: Dying due to exception : " + t.getMessage()); } } private boolean isIgnored(final Throwable t) { if (t instanceof IOException) { final String exceptionMessage = t.getMessage(); for (String ignoredMessage : IGNORED_IOEXCEPTION_MESSAGES) { if (exceptionMessage.startsWith(ignoredMessage)) { return true; } } } return false; } } |
and
public interface Console { void write(String msg); } |
Feel free to download the Java project.
