|
|
|
||
Let's say you've got a big class with a lot of methods -- a dispatcher or controller, say, that's gotten a bit out of control. The methods are real business methods (or at least they have business logic inside them), but they refer to several different entities, so you're not sure they belong on any given entity. But you want to get them out of the original class and into domain classes.
Moreover, these methods all do things in nearly the same way. In a controller, for instance, since they're all responding to different incoming events, they probably have similar code for handling exceptions, logging, maybe setting up and committing/rolling back database transactions, etc. This is a lot of functional duplication you'd like to remove.
Sounds like it might be time for a Command Pattern. Usually a Command is used when you want to encapsulate the doing of something, without actually doing it right away. But in this case, we'll execute it immediately. Don't worry, it will carry its own weight by removing the business logic from the original object, and by being cleaner and more isolated, and thus more testable. It will also be set up so that you can do all the cool Command pattern tricks later if you want, like polymorphic dispatch or undo.
Before:
public Money withdraw(int accountNumber, Money amount) throws SessionException {
try {
Account account = accountService.findAccount(accountNumber);
verifyBalance(account, amount);
account.withdraw(amount);
return account.getBalance();
}
catch (AccountServiceException e) {
logger.log(e);
throw new SessionException(e);
}
}
After:
public Money withdraw(final int accountNumber, final Money amount) throws SessionException {
return new Withdraw(accountNumber, amount, logger, accountService).execute();
}
...
class Withdraw extends AtmCommand<Money> {
private final int accountNumber;
private final Money amount;
private AccountService accountService;
public Withdraw(int accountNumber, Money amount, Logger logger, AccountService accountService) {
super(logger);
this.accountNumber = accountNumber;
this.amount = amount;
this.accountService = accountService;
}
protected Money doExecute() throws AccountServiceException, SessionException {
Account account = accountService.findAccount(accountNumber);
verifyBalance(account, amount);
account.withdraw(amount);
return account.getBalance();
}
}
In the rest of this article, I will walk through the precise sequence of steps to perform this refactoring in IntelliJ IDEA (version 5). There are some pretty fun keystroke combos you can use to make the whole thing relatively painless and automated. (Automating refactorings is better than just typing because it's less likely to introduce errors, it's got more stable intermediate points so you can safely stop halfway through, and most of all, because it makes you feel cool.)
Look at AtmCommand for a minute. Don't worry about all the details yet, but notice it uses a Template Method pattern to wrap the business logic in a try-catch block. Notice also that it uses Java 1.5 Generics to deal with the issue of different return values.
public abstract class AtmCommand<RETURN> {
protected final RETURN VOID = null;
private Logger logger;
protected AtmCommand(Logger logger) {
this.logger = logger;
}
final public RETURN execute() throws SessionException {
try {
return doExecute();
}
catch (AccountServiceException e) {
logger.log(e);
throw new SessionException(e);
}
catch (SessionException e) {
logger.log(e);
throw e;
}
}
protected abstract RETURN doExecute() throws AccountServiceException, SessionException;
}
Extract the body of the try block (Ctl-Alt-M). Just call it "foo".
public User checkOwner(int accountNumber) throws SessionException {
User result;
try {
result = foo(accountNumber);
}
catch (AccountServiceException e) {
logger.log(e);
throw new SessionException(e);
}
return result;
}
private User foo(int accountNumber) throws AccountServiceException {
User result;
Account account = accountService.findAccount(accountNumber);
result = account.getOwner();
return result;
}
Wrap the call to foo() with a creation and invocation of a new, anonymous inner Command class.
public User checkOwner(final int accountNumber) throws SessionException {
return new AtmCommand<User>(logger) {
protected User doExecute() throws AccountServiceException {
return foo(accountNumber);
}
}.execute();
}
Inline foo() (Ctl-Alt-N) so you can read it.
public User checkOwner(final int accountNumber) throws SessionException {
return new AtmCommand<User>(logger) {
protected User doExecute() throws AccountServiceException {
User result;
Account account = accountService.findAccount(accountNumber);
result = account.getOwner();
return result;
}
}.execute();
}
Put your cursor on the name "AtmCommand" and run "Convert anonymous to inner" (F6). Name it CheckOwner (or CheckOwnerCommand, if you're the type of person who names his pets "SpotDog" and "FelixCat").
public User checkOwner(final int accountNumber) throws SessionException {
return new CheckOwner(accountNumber).execute();
}
...
private class CheckOwner extends AtmCommand<User> {
private final int accountNumber;
public CheckOwner(int accountNumber) {
super(AtmSession.this.logger);
this.accountNumber = accountNumber;
}
protected User doExecute() throws AccountServiceException {
User result;
Account account = accountService.findAccount(accountNumber);
result = account.getOwner();
return result;
}
}
This is the trickiest step. Here we're removing all references to members of the original enclosing class. Fortunately the Java language has a quick way of at least identifying these references: make the inner class static. Once they go red, you can pull them up into the class as instance variables, and into the constructor as parameters, and up into the original caller, via a series of refactoring keystrokes.
private static class CheckOwner extends AtmCommand<User> {
private AccountService accountService;
private final int accountNumber;
public CheckOwner(Logger logger, AccountService accountService, int accountNumber) {
super(logger);
this.accountService = accountService;
this.accountNumber = accountNumber;
}
protected User doExecute() throws AccountServiceException {
User result;
Account account = accountService.findAccount(accountNumber);
result = account.getOwner();
return result;
}
}
Put your cursor on the name of the CheckOwner class, and run "Move..." (F6 again, just like "Convert anonymous to inner"). Select "inner to upper". Since it was already static, this should work with no errors.
Now it's your turn! Practice with the checkBalance() and deposit() methods.
The methods extracted so far were self-contained. Now let's see what happens with a method that calls other methods.
When we extract withdraw() we note that it calls the instance method
verifyBalance(). When we turn the extracted inner class static, this
call gets a red squiggly line underneath (meaning some sort of compile error).
Fortunately, IDEA has an Intention tailor-made for this case. Simply place your
cursor on the word verifyBalance and press Alt-Enter. A menu pops
up with "make 'verifyBalance' static". Select this and voilà!
Later we will move any static methods to a common base class of the relevant command classes. Usually what will happen is that you'll find that most methods are used by a subset of your commands, and that these methods tend to cluster together. You'll usually come away with a small number of distinct intermediate base classes, each of which has a few related methods used by most or all subclasses. This is a bottom-up way of identifying grouping patterns in your code that might otherwise have been hidden. This approach is an example of Zen Refactoring: let the code guide you to where it wants to go.
Once the command is an upper-level class, you will need to change the protection of the static method to "package". Again, there's an Intention for this.
Since transfer has a void return value, don't declare RETURN, and add "return VOID" at end of method. VOID is defined as a generic instance variable of the superclass. This is kind of a hack, but it makes the intention of the code clear.
class Transfer extends AtmCommand {
private final int fromAccountNumber;
private final int toAccountNumber;
private final Money amount;
private AccountService accountService;
public Transfer(int fromAccountNumber, int toAccountNumber, Money amount, Logger logger, AccountService accountService) {
super(logger);
this.fromAccountNumber = fromAccountNumber;
this.toAccountNumber = toAccountNumber;
this.amount = amount;
this.accountService = accountService;
}
protected Object doExecute() throws AccountServiceException, SessionException {
Account fromAccount = accountService.findAccount(fromAccountNumber);
Account toAccount = accountService.findAccount(toAccountNumber);
AtmSession.verifyBalance(fromAccount, amount);
fromAccount.withdraw(amount);
toAccount.deposit(amount);
return VOID;
}
}
I've left you with a class that looks a lot nicer,
but there's still more work to do. You can use IDEA's "Extract Superclass" refactoring
to create a common base class for Withdraw and Transfer,
then use "Move Method" to pull the verifyBalance method over,
then make verifyBalance non-static. You may also want to rearrange
parameters using "Change Signature" (Ctl-F6) to fit the semantics of the methods
rather than the haphazard order they appeared.
IDEA rocks.