Encapsulating a mutable object into a read-only object

Question

I'm currently implementing iterative solvers, which work by successively improving the estimate the solution to a specific problem. As the solution is a rather large set of data, refinement is carried out in place.

I have implemented a simple Observer/Observable pattern in order to be able to watch the algorithm while the iterations take place. In particular, the solver provides a method

Foo getCurrentSolution()

which returns the current estimate of the solution. The observer is then free to do some computations, based on the current estimate (for example: to decide whether or not the solution is good enough and the iterations could be stopped). Foo is mutable, but of course, if the observer modifies the current estimate of the solution, this can ruin the iterations of the solver.

Therefore, getCurrentSolution() should really return a defensive copy. But this requires time and memory on large problems, so I came up with another idea, which is to have getCurrentSolution() return a new ReadOnlyFoo(bar), where foo is the (mutable) current estimate of the solution, private to the solver. The idea is that ReadOnlyFoo has almost the same interface as Foo, only the methods which might modify the data are "deactivated" (they throw an exception). All details of some dummy classes are given below.

My question is: is this approach good practice? Is there a better pattern?

Thanks! Sebastien

public abstract class AbstractFoo{
    public abstract double getValue();

    public abstract void setValue(final double x);

    public abstract AbstractFoo add(AbstractFoo bar);

    public void addToSelf(AbstractFoo bar){
        setValue(getValue + bar.getValue());
    }
}

public class  Foo extends AbstractFoo{
    private double value;

    public Foo(final double x){
        value = x;
    }

    public double getValue(){
        return value;
    }

    public void setValue(final double x){
        value = x;
    }

    public AbstractFoo add(AbstractFoo bar){
        return new Foo(value + bar.getValue());
    }
}

public final class FooReadOnly extends AbstractFoo{
    private final Foo foo;

    public FooReadOnly(AbstractFoo foo){
        this.foo = foo;
    }

    public double getValue(){
        return foo.getValue();
    }

    public void setValue(final double x){
        throw new NotImplementedException("read only object");
    }

    public AbstractFoo add(AbstractFoo bar){
        return foo.add(bar);
    }

    public void addToSelf(AbstractFoo bar){
        throw new NotImplementedException("read only object");
    }
}

Answer 1

I would define an interface Solution containing only the read-only methods, and a mutable class MutableSolution containing all the methods, and make the getCurrentSolution() method return a Solution instance. This way, you don't need to create a defensive copy or to wrap your solution into a read-only wrapper.

Of course, the observer could still cast the solution to a MutableSolution, but this wouldn't be an accident. If you want to protect yourself against casts, then write a ReadOnlySolution wrapper class implementing Solution and delegating to the wrapped MutableSolution. This is similar to your proposition, except that the signature of the method makes it clear that the object is not mutable.

Answer 2

That's actually the approach that the Collections class does with unmodifiableList(...) etc. It returns a wrapper that contains the original list but throws exceptions in the methods that modify the collection.

Answer 3

I wouldn't do that. If one uses AbstractFoo of even a common interface (which might exist in your real implementation), then he doesn't know in advance, if the current instance is mutable or not. So the user will risk some unchecked exceptions to be thrown.

And, for an immutable object, beeing unmodifiable it's not exceptional at all. In other words: I wouldn't use execption to signal, that one attempted to modify an instance of FooReadOnly.

At least I'd add an abstract method boolean isModifiable() to the abstract class AbstractFoo so that we can test, if we can modify the object. And in that case, we wouldn't need to throw exceptions - the implementation of the modifying methods could simply do nothing.

Answer 4

Why do such an overengineered solution? Why not have one class and readOnly boolean attribute? Then do checkWriteable() for each setter.