Refactoring Teaser IV Solution
Its been a while since the Fourth Refactoring Teaser was posted. So far, I think this is one of the trickiest refactorings I’ve tried. Refactored half of the solution and rewrote the rest of it.
Particularly thrilled about shrinkage in the code base. Getting rid of all those convoluted Strategies and Child Strategies with 2 main classes was real fun (and difficult as well). Even though the solution is not up to the mark, its come a long long way from where it was.
Ended up renaming IdentityGenerator to EmailSuggester. Renamed the PartialAcceptanceTest to EmailSuggesterTest. Also really like how that test looks now:
28 29 30 | private final User naresh_from_mumbai = new User("naresh", "jains", "mumbai", "india", "indian"); private final Context lets = new Context(userService, dns); private final EmailSuggester suggester = new EmailSuggester(userService, dns, randomNumberGenerator); |
32 33 34 35 36 | @Test public void suggestIdsUsingNameLocationAndNationality() { List<String> suggestions = suggester.optionsFor(naresh_from_mumbai); lets.assertThat(suggestions).are("[email protected]", "[email protected]", "[email protected]", "[email protected]"); } |
38 39 40 41 42 43 | @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]"); } |
45 46 47 48 49 50 | @Test public void avoidCelebrityNamesInGeneratedIds() { lets.assume("naresh", "jains").isACelebrityName(); List<String> suggestions = suggester.optionsFor(naresh_from_mumbai); lets.assertThat(suggestions).are("[email protected]", "[email protected]", "[email protected]", "[email protected]"); } |
52 53 54 55 56 57 | @Test public void appendCurrentYearWithFirstNameIfIdIsNotAvailable() { lets.assume().identity("[email protected]").isNotAvailable(); List<String> suggestions = suggester.optionsFor(naresh_from_mumbai); lets.assertThat(suggestions).are("[email protected]", "[email protected]", "[email protected]", "[email protected]"); } |
EmailSuggester’s optionsFor() method turned out to be fairly straightforward.
26 27 28 29 30 31 32 33 34 | public List<String> optionsFor(final User user) { List<String> ids = new ArrayList<String>(); List<String> variations = asList(user.lastName, user.countryName, user.countryMoniker, user.city); for (String variation : variations) { UserData data = new UserData(user.firstName, variation, user.lastName); data.addGeneratedIdTo(ids); } return ids; } |
This method uses UserData class’ addGeneratedIdTo() method to add an email id to the list of ids passed in.
47 48 49 50 51 52 53 54 55 | private void addGeneratedIdTo(final List<String> ids) { for (EmailData potential : buildAllPotentialEmailCombinations()) { String email = Email.create(potential.userName, potential.domain, dns); if (userService.isEmailAvailable(email)) { ids.add(email); break; } } } |
This method fetches all potential email address combination based on user data as follows:
57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 | private List<EmailData> getAllPotentialEmailCombinations() { return new ArrayList<EmailData>() { { add(new EmailData(firstName, seed)); if (seed != lastName) { add(new EmailData((firstName + lastName), seed)); add(new EmailData((firstName + lastName.charAt(0)), seed)); } add(new EmailData((firstName + currentYear()), seed)); if (seed != lastName) add(new EmailData((firstName + lastName.charAt(0) + currentYear()), seed)); for (int i = 0; i < MAX_RETRIES_FOR_RANDOM_NUMBER; ++i) add(new EmailData((firstName + randomNumber.next()), seed)); } }; } |
I’m not happy with this method. This is the roughest part of this code. All the
if (seed != lastName) { |
seems dodgy. But at least all of it is in one place instead of being scattered around 10 different classes with tons of duplicate code.
For each potential email data, we try to create an email address, if its available, we add it, else we move to the next potential email data, till we exhaust the list.
Given two tokens (user name and domain name), the Email class tries to creates an email address without Restricted Words and Celebrity Names in it.
30 31 32 33 34 35 | private String buildIdWithoutRestrictedWordsAndCelebrityNames() { Email current = this; if (isCelebrityName()) current = trimLastCharacter(); return buildIdWithoutRestrictedWordsAndCelebrityNames(current, 1); } |
37 38 39 40 41 42 43 44 45 46 | private String buildIdWithoutRestrictedWordsAndCelebrityNames(final Email last, final int count) { if (count == MAX_ATTEMPTS) throw new IllegalStateException("Exceeded the Max number of tries"); String userName = findClosestNonRestrictiveWord(last.userName, RestrictedUserNames, 0); String domainName = findClosestNonRestrictiveWord(last.domainName, RestrictedDomainNames, 0); Email id = new Email(userName, domainName, dns); if (!id.isCelebrityName()) return id.asString(); return buildIdWithoutRestrictedWordsAndCelebrityNames(id.trimLastCharacter(), count + 1); } |
Influenced by Functional Programming, I’ve tried to use Tail recursion and Immutable objects here.
Also to get rid of massive duplication in code, I had to introduce a new Interface and 2 anonymous inner classes.
5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 | public interface RestrictedWords { RestrictedWords RestrictedUserNames = new RestrictedWords() { @Override public boolean contains(final String word, final DomainNameService dns) { return dns.isRestrictedUserName(word); } }; RestrictedWords RestrictedDomainNames = new RestrictedWords() { @Override public boolean contains(final String word, final DomainNameService dns) { return dns.isRestrictedDomainName(word); } }; boolean contains(final String word, DomainNameService dns); } |
This should give you a decent idea of what the code does and how it does what it does. To check in detail, download the complete project source code.
Also I would recommend you check out some the comparison of code before and after.
