Categories
Design Java

The Dangers of Escapism

I’m the kind of guy that has a ‘word of the day’ calendar and will use odd words during a conversation in an attempt to use the word sometime during the day. I especially enjoy when I learn a new word (or a new meaning of a word) that succinctly describes a concept or thought that I’ve been grasping to articulate. One such word that I recently came across describes a software design problem: “escapism.” Now I haven’t found a rigorous text book definition of the word, but I’ll do my best to define it:

Escapism: In software design, the inadvertent release of an object reference outside its current scope.

The usage of the qualifier “inadvertent” here is important because the programmer would probably find such behavior to be undesirable, though not always obvious.

One common source of escaped objects is the release of internal object references to external code. Programmers will work long and hard to encapsulate any internal objects a class uses in an effort to provide confidence in the object’s state, usage, etc. Anything that undermines that confidence should be addressed.

So, let’s look at a detailed example at where this form of escaping objects can cause problems.

Imagine a programmer creates a class—UserCache—to cache User objects. Part of the purpose of the class is that it will notify listeners when a User is added to or removed from the cache. To provide more flexibility, the programmer chooses to utilize the Decorator pattern to enhance an existing data structure with his cache logic rather than creating it from scratch.

Let’s see what the class looks like:

    public class UserCache extends AbstractSet<User> {

        private Set<User> rawSet;

        public UserCache(Set<User> rawSet) {
            this.rawSet = rawSet;
        }

        @Override
        public Iterator<User> iterator() {
            return rawSet.iterator();
        }

        @Override
        public boolean add(User user) {
            boolean wasAdded = rawSet.add(user);
            notifyListeners(user);
            return wasAdded;
        }

        @Override
        public boolean remove(Object o) {
            boolean wasRemoved = rawSet.remove(o);
            notifyListeners((User)o);
            return wasRemoved;
        }

    }

This is pretty standard use of the Decorator pattern. The programmer has decorated a standard java.util.Set named rawSet with some listener notification logic.

In general, whenever a class decorates another object it’s important (usually imperative) that the decoratee not be used for other purposes. For instance, adding or removing objects to the decoratee directly would be problematic:

    Set<User> internalSet = new HashSet<User>(64);
    Set<User> users       = new UserCache(internalSet);
    . . .
    internalSet.add(someUser);
    internalSet.add(anotherUser);

It is clearly inappropriate to use internalSet in this manner after it has been decorated as we would not call our listeners about these additions. As the creator of the UserCache class, we might not have any idea what sort of code depends on the listener notifications to work correctly. The result of missed notifications could result in any number of really bad problems. So it’s important we make the UserCache class bulletproof in terms of delivering event notifications.

Worried by this possibility the programmer rewrites the UserCache to use an internal java.util.Set rather than decorating a user-provided Set:

    public class UserCache extends AbstractSet<User> {

        private Set<User> rawSet;

        public UserCache() {
            rawSet = new HashSet<User>();
        }

        . . .

    }

So while it isn’t as flexible, at least now there’s no way that a user can get access to the internal java.util.Set and add or remove objects directly, right? As we look over the class we don’t see anywhere that rawSet is returned or passed as a parameter or anything. It would appear that the programmer has solved the problem. While there isn’t anywhere that we return rawSet directly, is there a way that we could be returning another object that will allow access to rawSet?

Take a closer look at the iterator method:

    @Override
    public Iterator<User> iterator() {
        return rawSet.iterator();
    }

Notice that it returns a java.util.Iterator based on rawSet. ‘But,’ you might say, ‘I want to be able to iterate over all the users.’ That’s fine. But recall that the java.util.Iterator interface includes a method named remove that removes the object last returned from next. What happens when we call remove is that the object is removed from rawSet directly – not from UserCache. In this case, UserCache would not receive any notification of the removal and thus it has no possible way to notify the listeners. While we would agree that most callers of iterator do simply want to iterate over the users, there’s no way to guarantee that some caller, some time, may try to remove a user directly via that interface.

In this example we would say that rawSet has escaped via the iterator method. This is a slight variation of escapism because strictly speaking we aren’t returning a reference to rawSet to external code, but we are returning a view of rawSet through which we can modify it. Unfortunately this is a fairly simple example. Take a look at the java.util.List and java.util.Map interfaces and see how many places a view of the internal object is returned. And note too the java.util.ListIterator that extends Iterator and includes an add and a set method 🙁

At this point it should be emphasized that it isn’t just java.util.Collection-related classes that need to be worried about escapism. The following code snippet shows how an object reference can escape through the constructor:

    public class FileRepository implements FileEventListener {

        private File directory;

        public FileRepository(File directory) {
            FileEventListenerManager.register(this);
            this.directory = directory;
        }

        public void fileAccessed(FileEvent fileEvent) {
            File updatedFile = fileEvent.getFile();

            if (directory.equals(updatedFile.getParentFile()))
                touch(updatedFile);
        }

        . . .

    }

In this second example, we have a class—FileRepository—that listens for accesses to files and touches them if they’re within a certain directory. Notice that constructor lets the this reference escape through the call to FileEventListenerManager.register(). While this may seem innocuous (and arguably intentional), note that the internal directory object has not been initialized. In a multithreaded environment it is perfectly possible that immediately after the listener registration occurs that a file is accessed, thus calling the FileRepository‘s fileAccessed method. If this occurs, a java.lang.NullPointerException will be thrown when the equals method is invoked on a then-null object.

As you can begin to imagine, there are many scenarios in which escapism can cause problems. So, how do we solve the problem of escapism?

In our UserCache example, we have to implement iterator as it’s part of the java.util.Set interface. A first shot might be to provide an implementation of java.util.Iterator that calls back to rawSet on calls to Iterator.remove. While this may seem like a workable solution on the surface, when Iterator.remove is called and then rawSet.remove is called, the caller will receive a java.util.ConcurrentModificationException because he’s both iterating and modifying rawSet “concurrently.”

Another possibility would be to return a java.util.Iterator from a read-only view of rawSet via:

    @Override
    public Iterator<User> iterator() {
        return Collections.unmodifiableSet(rawSet).iterator();
    }

The caller can still call Iterator.remove, but he will receive an java.lang.UnsupportedOperationException. While it’s arguable that teasing the caller with a method that will only fail in this way is bad design, at least it’s consistently so with other parts of the JDK.

In the example of the FileRepository it may seem like the easist thing to do is switch the code in the constructor to read:

    public class FileRepository implements FileEventListener {

        private File directory;

        public FileRepository(File directory) {
            this.directory = directory;
            FileEventListenerManager.register(this);
        }

        public void fileAccessed(FileEvent fileEvent) {
            File updatedFile = fileEvent.getFile();

            if (directory.equals(updatedFile.getParentFile()))
                touch(updatedFile);
        }

        . . .

    }

Initializing the directory object first will make a big difference. However, in other cases it might not be so cut and dry, especially in concurrent applications. The book Java Concurrency in Practice succinctly and bluntly advises:

Do not allow the this reference to escape during construction.

Some suggestions for this case are to create a static factory method such that the object can be fully constructed (and thus ensuring that its object reference is valid) before being handed off to any external code:

    public class FileRepository implements FileEventListener {

        private File directory;

        private FileRepository(File directory) {
            this.directory = directory;
        }

        public void fileAccessed(FileEvent fileEvent) {
            File updatedFile = fileEvent.getFile();

            if (directory.equals(updatedFile.getParentFile()))
                touch(updatedFile);
        }

        public static FileRepository create(File directory) {
            FileRepository fr = new FileRepository(directory);
            FileEventListenerManager.register(fr);
            return fr;
        }

        . . .

    }

Finding possible escape paths for objects takes serious attention to detail, knowing a little about the internals of the objects in use, and, of course, testing. But finding the problem is only half the battle, patching the escape paths is equally challenging. Unfortunately there is no across-the-board way to fix the problems, it takes hunkering down and thinking through the seeming myriad of possibilities.

Happy hunting! 😉