becomeImmutable?
Sorry for the sorry subject line, could not think of anything better.
I am working with an architecture that has two layers. Let's call it Outer and Inner. Outer layer provides "command"s to the Inner layer, which then processes them and provides the "result"s back to the Outer layer.
The commands are quire complex. Several methods in different classes participate in constructing the command and parameters and so on.
One (hypothetical?) constraint is that the Inner layer should not modify the command, only access it, perform the processing and create the Result object.
How do we enforce this constraint? One way is to create object of a completely different, immutable type, which is constructed from the Request object and passed on to the Inner layer. Another is to add a becomeImmutable method (or seal method) such that any mutating calls after that result in Exception. This moves the checking from compile time to run-time; but it is not as bad as it sounds. The methods in the Inner layer are not supposed to call the mutating methods and are violating contract by doing that.
Does this sound reasonable or am I completely wrong here? Has anyone done anything like this?
Thanks in advance for your valuable response.
# 1
Make the outer layer data objects implement an interface. Put
no modification methods on the interface. Make the inner layer
methods only accept the interface, not the object directly. Now the
inner layer cannot make any changes. The problem with
this solution is that now the inner layer is enforcing the
"cannot change" aspect of the command object, where it's really
the outer layer's job. If possible, make the command object
package scope so it can't leak out.
Better would be to make the command objects immutable,
if possible.
# 2
How will the inner layer process the query objects? Do these have complex, mutable objects as values that are accessed by the inner layer? If so, you need put in a bit of work in this. Do the different participants who create the query operate on the same values or do they have distinct areas of application? In the latter case, each participant could ensure to add immutable values to the query object, in the first case, you would have to "seal" the query before handing it into the inner layer.
Btw., does your need of immutability include the outer layer, once the query is delivered? In this case, you would have to create a deep copy of the query object. If not, you might either use the Interface approach if the provided values are non-mutable, or use a wrapper approach. The latter is similar to the Interface approach, but provides means to wrap inner values into wrappers, too, before handing them over to a user.
# 3
> Make the outer layer data objects implement an
> interface. Put
> no modification methods on the interface. Make the
> inner layer
> methods only accept the interface, not the object
> directly. Now the
> inner layer cannot make any changes.
If the concrete type is public, the type can be cast to it and the methods called. Before you say it, yes it's bad when someone downcasts the interface in this way but when you use this approach, you may actually encourage this kind of thing. Other developers may not understand the design and do this as a shortcut to get things done.
# 4
> How do we enforce this constraint? One way is to
> create object of a completely different, immutable
> type, which is constructed from the Request object
> and passed on to the Inner layer. Another is to add a
> becomeImmutable method (or seal method) such that any
> mutating calls after that result in Exception.
You can also create a wrapper that blocks the calls to the mutators (see Collections.UnmodifiableMap for an example.)
As was mentioned above, you can also use package scope to make the mutators to hide the mutators outside of the pacakge. Your packages have to be set up in a certain way for this to work and a lot of projects don't do it that way.
In addition to the first option, you can build two versions of the interface to the type, one immutable and a mutable one that extends it. Then you can use an immutable wrapper and avoid having to implement the mutators with runtime exceptions.
# 5
Thank you all for your responses.
Having a separate immutable type for the Inner layer is of course an option but it is much more suitable for new development. There is a lot of code out there and even though the change is practically automatic I don't think I can sell that. If I had to, I think I would prefer wrapper/delegation approach, though package (and in future module) level visibility would help if I were to use inheritance.
But my question is really on the 'seal' approach. Perhaps this was not very clear in the original post.
Does anyone have any experience/opinions on sealing a previously mutable object so that future (yes, I know temporal dependencies are bad) mutating calls throw , say, IllegalStateException?
Again, thank you for taking the time to think about this .
# 6
Opinion: I think it's bad design.
Experience: None, really. See above.
Implementation is trivial:
public class LockedClass {
private boolean isLocked = false;
private int x = 0;
public LockedClass() {
}
public void setX(final int x)
throws IllegalStateException {
if (this.isLocked) {
throw new IllegalStateException("This class is locked!");
}
this.x = x;
}
protected void setLocked() {
this.isLocked = true;
}
}
Message was edited by:
es5f2000
# 7
> Does anyone have any experience/opinions on sealing a
> previously mutable object so that future (yes, I know
> temporal dependencies are bad) mutating calls throw ,
> say, IllegalStateException?
I agree with the above. It's bad. I'm not sure I've seen that exact thing but I've seen a lot of code like it. What advantage does it provide over the wrapper solution? How you you make it mutable again?
Here's the solution I would go with:
interface Immutable
{
int getX();
}
interface Mutable extends Immutable
{
void setX(int x);
}
class ConcreteMutable implements Mutable
{
private int x;
public int getX()
{
return x;
}
public void setX(int x)
{
this.x = x;
}
}
class ImmutableWrapper implements Immutable
{
private final Immutable wrapped;
public ImmutableWrapper(Immutable toWrap)
{
this.wrapped = toWrap;
}
public int getX()
{
return wrapped.getX();
}
}
# 8
As far as I understood, the OP cannot exchange the classes without high costs, as they (or the system using them) is already spread along customers or users. Hence, creating a wrapper is no option (unless it is possible to make the class become a wrapper and the constructing parts operate on new classes).
I'm not sure about the bad design opinions. The sealing mechanism is somewhat similar to permission-based access to values. Except that, once sealed, no one has write access to its values anymore. Maybe using some permission mechanism would make it "nicer", but it seems oversized to me.
The wrapper example only works correctly as long as the immutable again contains immutable objects. Otherwise it could become quite a pain to write wrappers for all kind of mutable subvalues. But the same holds for sealing.
The clean way: refactor your classes to be wrappers and make the constructing classes operate on new classes, which are handed out using the wrappers.
The complex way: implement some permission mechanism that checks, whether the caller of a setter has write access.
The dirty way: seal setters by some locking mechanism.
# 9
> As far as I understood, the OP cannot exchange the
> classes without high costs, as they (or the system
> using them) is already spread along customers or
> users. Hence, creating a wrapper is no option (unless
> it is possible to make the class become a wrapper and
> the constructing parts operate on new classes).
If the OP considers "create object of a completely different, immutable type, which is constructed from the Request object and passed on to the Inner layer" a possible solution, I don't see how wrappers would not work.
> I'm not sure about the bad design opinions. The
> sealing mechanism is somewhat similar to
> permission-based access to values. Except that, once
> sealed, no one has write access to its values
> anymore. Maybe using some permission mechanism would
> make it "nicer", but it seems oversized to me.
It's bad because the code is overly complex. The wrapper solution can create the exact same behavior with better separation of concerns. I also find this solution to be incorrect because the 'Inner' subsystem never should call these methods so making them avaiable to the Inner subsystem only to make runtime exceptions possible is beyond pointless.
> The wrapper example only works correctly as long as
> the immutable again contains immutable objects.
> Otherwise it could become quite a pain to write
> wrappers for all kind of mutable subvalues. But the
> same holds for sealing.
I'm not sure what you mean here. Just to be clear, in the example, the Mutable extends the Immutable type. These are just example names of course. If you are saying that you'd need to create a wrapper for each distinct type, yes you would but if you use a tool like eclipse, it will do this for you.
# 10
> If the OP considers "create object of a completely
> different, immutable type, which is constructed from
> the Request object and passed on to the Inner layer"
> a possible solution, I don't see how wrappers would
> not work.
I was refering to this part: "Having a separate immutable type for the Inner layer is of course an option but it is much more suitable for new development. There is a lot of code out there and even though the change is practically automatic I don't think I can sell that."
> It's bad because the code is overly complex. The
> wrapper solution can create the exact same behavior
> with better separation of concerns. I also find this
> solution to be incorrect because the 'Inner'
> subsystem never should call these methods so making
> them avaiable to the Inner subsystem only to make
> runtime exceptions possible is beyond pointless.
I agree. But if I understood the above quote correctly, the "inner" subsystem already is using these classes, and exchanging classes (as is necessray with applying wrappers) would affect "a lot of code out there". I'm not sure about why to throw exceptions at all, calls to setters could simply get ignored.
> If you are saying that you'd need to create a wrapper
> for each distinct type, yes you would but if you use
> a tool like eclipse, it will do this for you.
That's just what I intended to say, thanks. A wrapper would have to wrap its internal variables, if these are mutable, and one would have to create an immutable wrapper for each of those variable types. Of course, their creation is supported by IDEs, but it may be difficult to cope wrt. their usages in the "inner" subsystem. For example, if one of the variable is of type Date, one would have to wrap Date and each access to it might have to be refactored. With Date, one may povide a clone for simplicity, but there are more complex types out there.
If this is not an issue, and all variables are immutable, I can only repeat myself and suggest to refactor the passed object to implement wrappers on the actual mutable ones, even if the setters become stubs or throw UnsupportedOperationExceptions.
# 11
> > If the OP considers "create object of a completely
> > different, immutable type, which is constructed
> from
> > the Request object and passed on to the Inner
> layer"
> > a possible solution, I don't see how wrappers
> would
> > not work.
>
> I was refering to this part: "Having a separate
> immutable type for the Inner layer is of course an
> option but it is much more suitable for new
> development. There is a lot of code out there and
> even though the change is practically automatic I
> don't think I can sell that."
Ah, OK. But as I think you were suggesting the original types could be replaced with interfaces. If the setters are not being used in the 'Inner' subsystem, removing them should not cause any problems. If the 'Inner' subsystem is using these, then the locking solution will probably cause a runtime failure. I'd rather deal with the compile time issue, personally.
Even if the locking solution is required, the wrapping verison is still cleaner. For one it's a lot easier to generate the code into a separate class which is what will probably eventually end up being done.
> > It's bad because the code is overly complex. The
> > wrapper solution can create the exact same
> behavior
> > with better separation of concerns. I also find
> this
> > solution to be incorrect because the 'Inner'
> > subsystem never should call these methods so
> making
> > them avaiable to the Inner subsystem only to make
> > runtime exceptions possible is beyond pointless.
>
> I agree. But if I understood the above quote
> correctly, the "inner" subsystem already is using
> these classes, and exchanging classes (as is
> necessray with applying wrappers) would affect "a lot
> of code out there". I'm not sure about why to throw
> exceptions at all, calls to setters could simply get
> ignored.
As I mentioned above, removing the setters from the classes will only affect the 'Inner' subsystem if it's actually calling these things. If it is, a no-op may be a good solution unless there's something that depends on this even though it's against the contract.
> > If you are saying that you'd need to create a
> wrapper
> > for each distinct type, yes you would but if you
> use
> > a tool like eclipse, it will do this for you.
>
> For example, if one of the
> variable is of type Date, one would have to wrap Date
> and each access to it might have to be refactored.
> With Date, one may povide a clone for simplicity, but
> there are more complex types out there.
I see now what you mean. But this is a problem if you use the locking solution too. The classes could still have a getImmutable method that just returns the wrapper and handles any recursive application of wrappers.
These kinds of situations are where dynamic hashtable style OO languages really shine. You could write a single class to deal with this for all your custom types in Python.
# 12
>
> If the 'Inner' subsystem is using these,
> then the locking solution will probably cause a
> runtime failure. I'd rather deal with the compile
> time issue, personally.
>
I suspect that if existing code is using them already then the only solution is a permission type system. Then the 'known' problem areas are allowed access while all others are denied.
# 13
Thanks a lot everyone for a very educating discussion; especially appreciate the explanation of what are flaws of sealing based solution. I am beginning to realize a wrapper based approach is the way to do this.Can someone please explain how I would implement permission based
# 14
> Thanks a lot everyone for a very educating
> discussion; especially appreciate the explanation of
> what are flaws of sealing based solution. I am
> beginning to realize a wrapper based approach is the
> way to do this.
>
> Can someone please explain how I would implement
> permission based approach?
I see two solutions.
The bad: take the caller as a parameter to the method and verify the class. The problem is that you don't know for sure that the caller passed in 'this'.
The ugly: capture the stack trace and see who called the method.
Perhaps jschell or stefan has a better solution.
# 15
If you expect to have issues applying the sealing approach, a permission approach will not help. First, you should analyze, what types of values are used in your request object. If it's only primitives and Strings, don't worry about it too much. For many types, you can hand out clones (should be deep clones though or only containing immutable element). For your own types, you can refactor them to become wrappers.
@dubwai: regarding ugly ... the Java permission system in parts is based on stack trace analysis, too. Not saying it's not ugly, though. ;)
# 16
> @dubwai: regarding ugly ... the Java permission
> system in parts is based on stack trace analysis,
> too. Not saying it's not ugly, though. ;)
Well, dumping the stacktrace used to require throwing an exception which used to be really expensive. It may not be a big deal anymore. But I still don't want to do it. I'd rather enforce rules like this through the API design but, of course, that's not always possible.
# 17
> Well, dumping the stacktrace used to require throwing
> an exception which used to be really expensive. It
> may not be a big deal anymore. But I still don't
> want to do it. I'd rather enforce rules like this
> through the API design but, of course, that's not
> always possible.
Well no, permission checks run via the SecurityManager which accesses the current execution stack natively. There is no other way for non-invasive checking on permissions, I guess. Of course, one could hand over some signed key as parameter, but that would force to change each accessors signature. If the accessors of outer and inner subsystems run in separate threads, maybe one could use signed thread implementations or similar. (Threads provide native stack trace access, too, btw.)
