Cleaning up your code with real objects, dependency injection and aspects

Over the past year I've given a few presentations on how to clean up those bloated service classes that are sadly all to common in Java EE applications. A real-world example of this kind of service would be too large to include in a blog so here is an example that is only a minor mess:

public class AccountServiceProceduralImpl implements AccountService {

  private Log logger = LogFactory.getLog(getClass());

  private final AccountDao accountDao;

  private final BankingTransactionDao bankingTransactionDao;

  public AccountServiceProceduralImpl() {

    this.accountDao = new JdbcAccountDao();

    this.bankingTransactionDao = new JdbcBankingTransactionDao();

  }

  public BankingTransaction transfer(String fromAccountId, String toAccountId,

      double amount) {

    BankingSecurityManager.verifyCallerAuthorized(AccountService.class,

        "transfer");

    logger.debug("Entering AccountServiceProceduralImpl.transfer()");

    TransactionManager.getInstance().begin();

    AuditingManager.getInstance().audit(AccountService.class, "transfer",

        new Object[] { fromAccountId, toAccountId, amount });

    try {

      Account fromAccount = accountDao.findAccount(fromAccountId);

      Account toAccount = accountDao.findAccount(toAccountId);

      double newBalance = fromAccount.getBalance() - amount;

      switch (fromAccount.getOverdraftPolicy()) {

      case Account.NEVER:

        if (newBalance < 0)

          throw new MoneyTransferException("Insufficient funds");

        break;

      case Account.ALLOWED:

        Calendar then = Calendar.getInstance();

        then.setTime(fromAccount.getDateOpened());

        Calendar now = Calendar.getInstance();

        double yearsOpened = now.get(Calendar.YEAR) - then.get(Calendar.YEAR);

        int monthsOpened = now.get(Calendar.MONTH) - then.get(Calendar.MONTH);

        if (monthsOpened < 0) {

          yearsOpened--;

          monthsOpened += 12;

        }

        yearsOpened = yearsOpened + (monthsOpened / 12.0);

        if (yearsOpened < fromAccount.getRequiredYearsOpen()

            || newBalance < fromAccount.getLimit())

          throw new MoneyTransferException("Limit exceeded");

        break;

      default:

        throw new MoneyTransferException("Unknown overdraft type: "

            + fromAccount.getOverdraftPolicy());

      }

      fromAccount.setBalance(newBalance);

      toAccount.setBalance(toAccount.getBalance() + amount);

      accountDao.saveAccount(fromAccount);

      accountDao.saveAccount(toAccount);

      TransferTransaction txn = new TransferTransaction(fromAccount, toAccount,

          amount, new Date());

      bankingTransactionDao.addTransaction(txn);

      TransactionManager.getInstance().commit();

      logger.debug("Leaving AccountServiceProceduralImpl.transfer()");

      return txn;

    } catch (MoneyTransferException e) {

      logger.debug(

          "Exception thrown in AccountServiceProceduralImpl.transfer()", e);

      TransactionManager.getInstance().commit();

      throw e;

    } catch (RuntimeException e) {

      logger.debug(

          "Exception thrown in AccountServiceProceduralImpl.transfer()", e);

      throw e;

    } finally {

      TransactionManager.getInstance().rollbackIfNecessary();

    }

  }

  public void create(Account account) {

    BankingSecurityManager.verifyCallerAuthorized(AccountService.class,

        "create");

    logger.debug("Entering AccountServiceProceduralImpl.create()");

    TransactionManager.getInstance().begin();

    AuditingManager.getInstance().audit(AccountService.class, "create",

        new Object[] { account.getAccountId() });

    try {

      accountDao.addAccount(account);

      TransactionManager.getInstance().commit();

      logger.debug("Leaving AccountServiceProceduralImpl.create()");

    } catch (RuntimeException e) {

      logger.debug("Exception thrown in AccountServiceProceduralImpl.create()",

          e);

      throw e;

    } finally {

      TransactionManager.getInstance().rollbackIfNecessary();

    }

  }

}

This code has a few characteristics that make it difficult to maintain and test:

• Tight coupling - it directly instantiates the DAOs and accesses other infrastructure related classes through static methods or singletons.
• Tangling - the service contains a mixture of business logic and infrastructure (security/transactions/auditing/logging/etc) code.
• Scattering - the infrastructure logic is duplicated in every service method
• Anemic domain model - the "domain objects" are dumb data objects - all of the business logic is concentrated in the bloated service class

Compare and contrast this with a service from version of the same code that uses aspects, dependency injection and a real domain model:

public class AccountServiceImpl implements AccountService {

    private final AccountRepository accountRepository;

    private final BankingTransactionRepository bankingTransactionRepository;

    public MoneyTransferServiceImpl(AccountRepository accountRepository,

            BankingTransactionRepository bankingTransactionRepository) {

        this.accountRepository = accountRepository;

        this.bankingTransactionRepository = bankingTransactionRepository;

    }

    public BankingTransaction transfer(String fromAccountId,

            String toAccountId, double amount) throws MoneyTransferException {

        Account fromAccount = accountRepository.findAccount(fromAccountId);

        Account toAccount = accountRepository.findAccount(toAccountId);

        fromAccount.debit(amount);

        toAccount.credit(amount);

        TransferTransaction txn = new TransferTransaction(fromAccount,

                toAccount, amount, new Date());

        bankingTransactionRepository.addTransaction(txn);

        return txn;

    }

  public void create(Account account) {

    accountRepository.addAccount(account);

  }

}

As you can see there are some significant improvements:

• Simpler code - the service method is a lot smaller
  
• Improved separation of concerns - it does not contain any infrastructure-related logic
• Less coupled - it is written in terms of repository/dao interface
• More object-oriented - the service contains very little business logic: that's now in the Account class and elsewhere

To find out more take a look at these presentations:

• Improving application design with a rich domain model - versions of this talk we given at Spring One (video) , the Colorado Software Summit and the Spring Experience (slides and audio) .
• Simplifying code with dependency injection and aspects - versions of this talk were given at  SV Web JUG, the Colorado Software Summit and the SD Forum Java SIG.

tags: aop  aspects  spring  dependency  injection  rich  domain  model  design  

links: digg this    del.icio.us    technorati    reddit