Using the Composite class: how a client can tell if it is composite
I know there is a similar question here. He looked at a more general question about class behavior than my question here.
Consider the following simple implementation of the composite pattern:
interface Item {
int getWeight();
}
class SimpleItem implements Item {
private int weight;
public int getWeight() {
return weight;
}
}
class Container implements Item {
private List<Item> items;
private int weight;
public void add(Item item) {
items.add(item);
}
public int getWeight() {
return weight + items.stream().mapToInt(Item::getWeight).sum();
}
}
Now let's look at how the user of an element can determine if it is a container. For example, a container pushAdd
requires a method that pushes an item down the hierarchy into a container that has no containers. The container only knows about items, it doesn't know if these items are containers or SimpleItems or another class that implements Item.
There are three possible solutions:
1. Specimen usage and casting
public void pushAdd(Item item) {
Optional<Container> childContainer = items.stream()
.filter(item instanceof Container)
.map(item -> (Container)item)
.findAny();
if (childContainer.isPresent()) {
childContainer.get().pushAdd(item);
} else {
add(item);
}
}
2. Implementation is / as methods
public pushAdd(Item item) {
Optional<Container> childContainer = items.stream()
.filter(Item::isContainer)
.map(Item::asContainer);
....
}
3. Visitor template (I missed a simple implementation accept
).
interface ItemVisitor {
default void visit(SimpleItem simpleItem) { throw ...}
default void visit(Container container) { throw ... };
}
public pushAdd(Item item) {
Optional<Container> childContainer = ... (using instanceOf);
if (childContainer.isPresent()) {
childContainer.get().accept(new ItemVisitor(item) {
void visit(Container container) {
container.pushAdd(item);
}
};
} else {
add(item);
}
}
The first is evil because it uses instanceof and casting. The second is evil because it forces the knowledge of the Container into the Item - and it gets much worse when other subclasses of the item are created. The third will not help you know if you can add to the element before calling the visitor. You can catch the exception, but that seems like the wrong use of exceptions to me: it's much better to have a way to check before visiting.
So my question is, is there another pattern I can use to avoid casting and instantiating without having to pass knowledge of subclasses into the hierarchy?
source to share
I think I am talking for any java persona when I say that the visitor pattern is not very popular in java. So the above can be implemented like this (I'll be using interfaces here because in my experience they are more flexible):
interface Item { /* methods omitted */ }
interface SimpleItem extends Item { /* methods omitted */ }
interface ContainerItem extends Item { /* methods omitted */ }
// telling clients that this can throw an exception or not
// here is a whole different design question :)
interface ItemVisitor { void visit(Item i) /* throws Exception */; }
class MyVisitor implements ItemVisitor {
void visit(Item i) {
if (i instanceof SimpleItem) {
// handle simple items
} else if (i instanceof ContainerItem) {
// handle containers using container specific methods
} else {
// either throw or ignore, depending on the specifications
}
}
}
The cost is instanceof
pretty low on the latest JVM, so I won't worry too much about that unless you can prove that the traditional visitor is significantly faster.
The readability and maintainability of the code is possibly identical, with a few differences. First of all, if a new interface is added to the hierarchy that does not change existing visitors, those visitors do not need to change. On the other hand, it is easier to forget the visitor to change (especially in client code that you have no control over) because the visitor does not explicitly require the customers to do this, but hey, that's the nature of the code and the general design flaw that requires a visit.
Another benefit of this pattern is that customers who don't need a visit don't have to worry about it (no method accept
) i.e. shorter learning curve.
Finally, I think this pattern is closer to a "purist" OOD in the sense that the interface hierarchy does not contain false methods ( visit
, canAddItems
etc.), i.e. there are no "tags" .
source to share
Well, it seems due to the fact that no one posted the answer that there are no better options than the 3 I am putting.
So, I will post my preferred solution and see if anyone can improve it. In my opinion, the best option is a combination of options 2 and 3. I don't think it's too bad to have a member canAddItems
Item
- it might argue that it is reasonable that the Item developers tell you if you can add Items to them. But visitors seem to be a good way to hide the details of adding items.
So fwiw, this is my best compromise on the task I have posed. I am still not 100% happy with this. In particular, a visitor adding items will be broken if another class is implemented that can add items. But this is probably what you want, because it changes the semantics of pushAdd.
interface Item {
int getWeight();
void accept(ItemVisitor visitor);
default boolean canAddItems() {
return false;
}
}
interface ItemVisitor {
default void visit(SimpleItem simpleItem) {
throw new IllegalArgumentException("ItemVisitor does not accept SimpleItem");
}
default void visit(Container container) {
throw new IllegalArgumentException("ItemVisitor does not accept Container");
}
}
class SimpleItem implements Item {
private int weight;
public int getWeight() {
return weight;
}
public void accept(ItemVisitor visitor) {
visitor.visit(this);
}
}
class Container implements Item {
private List<Item> items;
private int weight;
public void add(Item item) {
items.add(item);
}
public int getWeight() {
return weight + items.stream().mapToInt(Item::getWeight).sum();
}
public void accept(ItemVisitor visitor) {
visitor.visit(this);
}
public void pushAdd(Item item) {
Optional<Item> child = items.stream().filter(Item::canAddItems).findAny();
if (child.isPresent()) {
child.get().accept(new ItemVisitor() {
public void visit(Container container) {
container.add(item);
}
});
} else {
add(item);
}
}
}
source to share