If you read Elegant Objects, you probably know about vertical decorator pattern. However, there are right ways and wrong ways to do it. In this post I want to propose a few additional rules I haven’t seen in the book, even though I think they are there implicitly. If you adhere to them, you are less likely to mess up your code. Once formulated, they may seem obvious, but we learned them the hard way.

1. Decorator compositions should be logic-free. Logic should be in decorators, not in composition.
2. Each decorator may have only one responsibility, in addition to delegating to the encapsulated object. They can be one of two types: a) Normal decorators. They take an object of their type and and add one additional piece of functionality: filtering, formatting, validations, etc. b) Decorating envelopes. They only build a decorator composition and delegate to it.

Let me explain with a story from a real project I and my colleague had been working on for several months. I will severely simplify both the code and the story to demonstrate the main point.

Transactions

A bank customer has several accounts, each having a list of transactions on it. A third party can access those transactions if the owner consents. The owner, when consenting, can chose which accounts he wants disclose, and whether the third party will be able to see only basic details of each transaction, or everything.

These are the transactions a third party can access with owner’s consent:

public final class Transactions {
    private final String accountId;
    private final LocalDate from;
    private final LocalDate to;
    private final Consent consent;
    private final TransactionsSource source;

    public Transactions(
        String accountId, LocalDate from, LocalDate to,
        Consent consent, TransactionsSource source
    ) {
        this.accountId = accountId;
        this.from = from;
        this.to = to;
        this.consent = consent;
        this.source = source;
    }

    public List<Transaction> list() {
        mustBeTrue(consent.isForAccount(accountId));
        mustBeTrue(
            consent.hasPermission(Permission.BASIC)
                || consent.hasPermission(Permission.DETAIL)
        );
        List<Transaction> transactions =  source.transactions(
            accountId, from, to
        );
        if (!consent.hasPermission(Permission.DETAIL)) {
            transactions.forEach(this::removeSpecialFields);
        }
        return transactions;
    }
    // ...
}

The consent has to be for the specified account. It must have any of the required permissions. If it doesn’t have DETAIL permission, special fields are filtered out. Seems simple enough.

Introducing decorators

Then we had to add more permissions, specifically for credit and debit transactions. At this point it seemed a good opportunity to refactor with a decorator for each permission. And so we did. We created the Resource (because we were working with many different resources, not just transactions) interface:

public interface Resource<T> {
    T value();
}

We moved validations into AuthorisedTransactions:

public final class AuthorisedTransactions implements Resource<List<Transaction>> {
    private final Resource<List<Transaction>> origin;
    private final String accountId;
    private final Consent consent;

    public AuthorisedTransactions(
        Resource<List<Transaction>> origin,
        String accountId,
        Consent consent
    ) {
        this.origin = origin;
        this.accountId = accountId;
        this.consent = consent;
    }

    @Override
    public List<Transaction> value() {
        mustBeTrue(consent.isForAccount(accountId));
        mustBeTrue(
            consent.hasPermission(Permission.BASIC)
                || consent.hasPermission(Permission.DETAIL)
        );
        return origin.value();
    }
    // ...
}

We moved details filtering into BasicTransactions:

public final class BasicTransactions implements Resource<List<Transaction>> {
    private final Resource<List<Transaction>> origin;

    public BasicTransactions(Resource<List<Transaction>> origin) {
        this.origin = origin;
    }

    @Override
    public List<Transaction> value() {
        List<Transaction> transactions = origin.value();
        transactions.forEach(this::removeSpecialFields);
        return transactions;
    }
    // ...
}

And we created two decorators to filter out credit or debit transactions:

public final class NoCreditTransactions implements Resource<List<Transaction>> {
    // ...
    @Override
    public List<Transaction> value() {
        List<Transaction> transactions = origin.value();
        transactions.removeIf(Transaction::isCredit);
        return transactions;
    }
}

public final class NoDebitTransactions implements Resource<List<Transaction>> {
    // ...
    @Override
    public List<Transaction> value() {
        List<Transaction> transactions = origin.value();
        transactions.removeIf(Transaction::isDebit);
        return transactions;
    }
}

Then the main Transactions class became:

public final class Transactions implements Resource<List<Transaction>> {
    // ...
    @Override
    public List<Transaction> value() {
        Resource<List<Transaction>> transactions = new AuthorisedTransactions(
            new CompleteTransactions(
                accountId, from, to, source
            ), accountId, consent
        );
        if (!consent.hasPermission(Permission.DETAIL)) {
            transactions = new BasicTransactions(transactions);
        }
        if (!consent.hasPermission(Permission.CREDIT)) {
            transactions = new NoCreditTransactions(transactions);
        }
        if (!consent.hasPermission(Permission.DEBIT)) {
            transactions = new NoDebitTransactions(transactions);
        }
        return transactions.value();
    }
}

There are several things that immediately smell here:

1. It’s procedural and imperative - it consists of several sequential conditional instructions on how to compose the decorators.
2. It keeps reassigning a local variable.
3. It instantiates objects in methods.

This is where the first rule should have been applied - decorator compositions should be logic-free. We didn’t have the rule at that time though. We saw that these instructions are not temporally coupled to each other - you can switch them around. This is less imperative that it would be without decorators. We could move the conditionals into decorators, but then they would become less cohesive and more configurable. And if we moved the code to constructor so as not to have objects instantiated in method, we would then have logic in constructor, which is also not good. Having all of that in mind we decided to leave it as it is and see if it works out.

Turning into mess

Then we decided to add a new decorator. The account ID which comes in with the actual request is UUID and needed to be mapped to our internal String representation before it was passed to the Transactions object:

String id = mapper.idForUuid(accountId);
List<Transaction> transactions =  new Transactions(id, ...

We already had momentum with decorators so we tried to solve it the same way - create a decorator which would enable Transactions to work with UUID instead of internal String ID. However, we couldn’t put it inside Transactions, like all others, because Transactions already depended on String ID. So we put Transactions inside it:

public final class TransactionsForUuid implements Resource<List<Transaction>> {
    private final Resource<List<Transaction>> transactions;

    public TransactionsForUuid(
        UUID accountId, LocalDate from, LocalDate to,
        Consent consent,
        TransactionsSource source, AccountIdMapper mapper
    ) {
        this.transactions = new Transactions(
            mapper.idForUuid(accountId),
            from, to, consent, source
        );
    }

    @Override
    public List<Transaction> value() {
        return transactions.value();
    }
}

This is where it really started bothering us. Our decorators had way too many dependencies. Look at this one. It’s only for account ID mapping, and yet it has 6 constructor parameters, most of which it just passes along. That is because (and this is where the second rules kicks in) it is a hybrid decorator. It has mixed responsibilities. It maps account IDs and it also constructs a new decorator composition it then encapsulates. So naturally it needs all of the dependencies necessary to map account IDs, plus all of the dependencies necessary to construct the whole Transactions composition. This is wrong. Decorators should either be normal decorators which take an object in constructor and perform one additional job after or before sending a message to that object, or decorating envelopes, which construct the whole composition and do nothing else. Then there will be only one decorator with all of the dependencies - envelope - and each normal decorator will have few dependencies - only the ones necessary to perform its single responsibility.

Applying the rules

We couldn’t refactor TransactionsForUuid while complying with the second rule, so we threw it away and introduced an AccountId value object. This is a better design anyway.

public interface AccountId {
    String internal();
    UUID external();
}

To comply with the first rule we moved the conditionals into decorators. E.g. NoCreditTransactions became:

public final class NoCreditTransactions implements Resource<List<Transaction>> {
    private final Resource<List<Transaction>> origin;
    private final Consent consent;

    public NoCreditTransactions(
        Resource<List<Transaction>> origin,
        Consent consent
    ) {
        this.origin = origin;
        this.consent = consent;
    }

    @Override
    public List<Transaction> value() {
        List<Transaction> transactions = origin.value();
        if (!consent.hasPermission(Permission.CREDIT)) {
            transactions.removeIf(Transaction::isCredit);
        }
        return transactions;
    }
}

And the Transactions class became a true decorating envelope:

public final class Transactions implements Resource<List<Transaction>> {
    private final Resource<List<Transaction>> transactions;

    public Transactions(
        AccountId accountId,
        LocalDate from, LocalDate to,
        Consent consent, TransactionsSource source
    ) {
        this.transactions = new AuthorisedTransactions(
            new NoCreditTransactions(
                new NoDebitTransactions(
                    new BasicTransactions(
                        new CompleteTransactions(
                            accountId, from, to, source
                        ),
                        consent
                    ), consent
                ), consent
            ), accountId, consent
        );
    }

    @Override
    public List<Transaction> value() {
        return transactions.value();
    }
}

Complying with these two rules solved all (almost all, see below) problems: minimized dependencies for normal decorators, no more procedural code, no variable reassignments, no object instantiation in methods and no logic in constructors.

Also, turns out these object “pyramids” are very useful once you get used to them. Once you learn to read them, they are like a model of the whole (or part of) application logic distilled in one class. When you open a new project, it’s like opening a map of a new city you just arrived in.

Caveat

There is still one smell left. NoCreditTransactions is not really NoCreditTransactions now, is it? It’s more like NoCreditIfNoPermissionWasGivenTransactions. The fact that it is difficult to name this object indicates this may not be the ideal design still. What do you think? How would you name it?

Conclusion

When building vertical decorators, if you want to end up with clean code, the following two rules from Elegant Objects are necessary:

1. Keep constructors code-free.
2. Don’t use new outside secondary constructors.

I propose two additional rules specifically for decorators, which were not explicitly stated, but I think are implicit in the book:

1. Decorator compositions should be logic-free. Logic should be in decorators, not in composition.
2. Each decorator may have only one responsibility, in addition to delegating to the encapsulated object. They may be one of two types: a) Normal decorators, which take an object of their type and add one additional piece of functionality. b) Decorator envelopes, which build a decorator composition and delegate to it.