A Case Of Leaky State

Leaky state is among the most common artifacts I find in existing applications. It's a symptom of poor encapsulation and/or abstraction and typically tends to unmaintainable code. Such symptoms are typically, but by no means exclusively, seen in an anemic domain model. Controlling state by means of encapsulation and abstraction is a decent way of taming coupling and complexity.

Real Life Example

Consider, for a moment, you owe me $5, and you have numerous ways to pay me: credit card, debit card, cash, cheque, etc. Imagine, now, I walk up to you and take your wallet out of your pocket. I proceed to take out $5 cash. Thankfully, I'm responsible and honest: I don't take anything else. Unfortunately, another less honest individual may have taken everything.

As my relationship to you, the reader, is likely that of a stranger, this behaviour is abhorrent and unacceptable. Indeed, you, being the honest person you are, would take out, from your own pocket, your own wallet and give me $5. Or, perhaps you would write me a cheque for $5. In this approach, there is no violation of privacy and you are able to pay in a means of your choosing.

A Java Example

By "leaky state," I refer to an object exposing various pieces of data that, in part or whole, make up an object's state. Consider, for instance, a very basic system that shows the state of a brick-and-mortar store. To keep this example simple, only one state is being modeled in this example: whether the store is open or closed for business. Assume this state is determined by three simple facts:

A typical Java implementation follows (realize this is not a comprehensive implementation):

public final class Store
{
    private TimeRange hoursOfBusiness;
    private boolean isOwnerAtStore;

    public TimeRange getHoursOfBusiness()
    {
        return this.hoursOfBusiness;
    }

    public boolean isOwnerAtStore()
    {
        return this.isOwnerAtStore;
    }

    //...
}

A typical example of the "system" component follows

public final class StoreStateService
{
    public void showStateForAllStores(final List<Store> stores)
    {
        final Date now = new Date();
        for(final Store store : stores){
            final boolean isInRangeOfHours = store.getHoursOfBusiness().intersects(now);
            final boolean isTodayAHoliday = this.holidays.contains(now);
            final boolean isOwnerAtStore = store.isOwnerAtStore();

            final String state;
            if(isInRangeOfHours && !isTodayAHoliday && isOwnerAtStore){
                state = "open";
            } else {
                state = "closed";
            }

            System.out.println(store + " is " + state);
        }
    }

    //...
}

What's Wrong?

Several things are "wrong" with this example. Notice how StoreStateService has a fair bit of coupling to Store. The service knows, intimately, how a Store's state is determined. While one may argue this is good, consider changes to Store. Assume, for instance, this system was a big success and went international. Not all countries and cultures follow the same holiday schedule. Further, how do time zones get factored in? The Store's hours are determined by the timezone in which the store resides. Can a big-box store not open without the owner being present?

We have to make system changes. Not only do we need to keep the existing business logic, but we have to add new business logic, too. The following lists our full system requirements

There are a few ways this could be done, but the following represents a fairly typical solution:

public final class Store
{
    private TimeRange hoursOfBusiness;
    private TimeZone timezone;
    private Map<Date> holidays;
    private boolean isOwnerAtStore;
    private boolean isOwnerRequired;

    public TimeRange getHoursOfBusiness()
    {
        return this.hoursOfBusiness;
    }

    public TimeZone getTimeZone()
    {
        return this.timezone;
    }

    public Map<Date> getHolidays()
    {
        return Collections.unmodifiableMap(this.holidays);
    }

    public boolean isOwnerAtStore()
    {
        return this.isOwnerAtStore;
    }

    public boolean isOwnerRequired()
    {
        return this.isOwnerRequired;
    }

    //...
}

The Store class is getting pretty bulky. Now we have to patch our business logic

public final class StoreStateService
{
    public void showStateForAllStores(final List<Store> stores)
    {
        final Date now = new Date(store.getTimeZone());
        for(final Store store : stores){
            final boolean isInRangeOfHours = store.getHoursOfBusiness().intersects(now);
            final boolean isTodayAHoliday = store.getHolidays().contains(now);

            final boolean isOwnerRequired = store.isOwnerRequired();
            final boolean isOwnerAtStore = store.isOwnerAtStore();

            final String state;
            if(isInRangeOfHours && !isTodayAHoliday
                && ((isOwnerRequired && isOwnerAtStore ) || !isOwnerRequired)){
                state = "open";
            } else {
                state = "closed";
            }

            System.out.println(store + " is " + state);

        }
    }

    //...
}

This service became much more complicated (the example quite likely has a logic bug or two). Alternatively, I've seen developers attempt to solve this issue with subclasses of Store and branch in StoreStateService with instanceof. Such implementations do not solve the state leakage and typically violate the Liskov Substitution Principle. Ultimately, matters become worse. Finally, I have also seen developers introducing a third class, StoreUtils. This still doesn't plug the leaky state issue, but it does introduce a whole lot more coupling.

Consider how a few more subtle changes to Store could impact the system. Perhaps we now have to check if the store owner has paid us to show the store's status. Not only must we change Store, but also StoreStateService and probably a few other classes. A Store isn't reusable even within its own system. This is why leaking state is less than ideal.

Solutions

Luckily, a bit of encapsulation and abstraction can solve this problem. Recall that that this system is "determining if a store is open or closed". Re-read that sentence a few times. Realize that the currently exposed attributes of Store are inconsequential to the system as a whole: the attributes are of importance to only the Store. When criteria change for a store to be open or closed, only Store should need to change. Consider the following

public abstract class Store
{
    private TimeRange hoursOfBusiness;
    private TimeZone timezone;
    private Map<Date> holidays;

    public boolean isOpen()
    {
        final Date now = new Date(this.timezone);
        return this.isOpenForDateAndTime(now);
    }

    private boolean isOpenForDateAndTime(final Date date)
    {
        final boolean isInRangeOfHours = this.hoursOfBusiness.intersects(now);
        final boolean isTodayAHoliday = this.holidays.contains(now);
        return isInRangeOfHours && isTodayAHoliday;
    }
}


public final class MomAndPopStore extends Store
{
    private boolean isOwnerAtStore;

    @Override
    public boolean isOpen()
    {
        return super.isOpen() && this.isOwnerAtStore;
    }
}


public final class BigBoxStore extends Store
{
}

The largest difference is that Store publicly exposes only the isOpen() method. All other details are available only to subclasses or to Store itself. The StoreStateService, for example, may now only invoke isOpen()--just one method.

Also, the requirements changed to indicate the system now models more than one type of store: a big-box store and the original mom-and-pop shop. Without separating the two entities, the implementation would become rife with fields applicable only to some types of Stores--a nightmare of complexity. Interestingly, BigBoxStore is an empty class. This fact may suggest that the abstraction is incorrect. Alternatively, it may be indicative of premature generalisation.

How do these changes impact StoreStateService? The following now shows an implementation that is sufficiently decoupled from the implementation of Store.

public final class StoreStateService
{
    public void showStateForAllStores(final List<Store> stores)
    {
        for(final Store store : stores){
            final String state = store.isOpen() ? "open" : "closed";
            System.out.println(store + " is " + state);
        }
    }
}

This service is now small and understandable. Store state logic is now where it semantically belongs and the service no longer need know the Store's full implementation. This example could go one step further and use an enum (or other similar mechanism) to communicate, in a type-safe fashion, "open" or "closed;" however, I will leave this as an exercise to you, should you wish.

What About C?

This approach is viable and encouraged in C. Though C is not an Object Oriented language, principles of encapsulation still exist. While I cannot currently profess an intimate understanding of C, using an opaque pointer allows state to remain hidden while logic is funneled through the correct function implementations.

Indeed, accessing a struct's members from other modules/units is just as abusive as accessing an object's instance fields (whether by accessor or not). Both tend to the same fragile code that is difficult to maintain.

What I'm Not Saying

Do not interpret my suggestion incorrectly. Exposing state is not inherently bad. In fact, exposing aggregate state is often a requirement and limiting its exposing may, in fact, lead to more complex design. However, my intention is to express that exposing too much fine-grained state encourages misuse of an API and often leads to an anemic domain model. If the invoker does not need to know some specific fact, do not expose it.

Embarrassingly, I admit I once took this too far. I refused to expose state beyond a protected scope and required the use of the Visitor Pattern to retrieve state. This was a very bad idea and I would discourage you from going down the same path.

Conclusion

No solution or system is perfect. Design decisions are, in general, made with a very limited understanding of a very large domain. Though encapsulation and abstraction help eliminate leaky state, no one-size-fits-all solution exists (to my knowledge). Indeed, other solutions exist.

I have worked with several developers that insist the original example is the optimal implementation since all business "rules" are held in one place. While I vehemently disagree with such an argument in most circumstances, the point is nevertheless valid. A system in which performance is an utmost requirement may benefit, when implemented correctly, from such a tightly coupled design (or so my understanding suggests). As far as I'm concerned, trading code clarity and maintainability for (potential) performance gains is a business risk decision and not a technical decision.

Let this be just one more tool in your toolbox.

Further Readings

A vast number of papers, books, websites, and talks have focused on this subject. The following lists a few subjects that pointed me along the right direction: