Problem
I am studying the composite pattern and have created a very simple project that uses composite to introduce family members. I have decided to go with the ‘fully transparent’ implementation by defining all of my common functions in the interface class, I feel this is the neatest implementation and am considering throwing an exception in Leaf classes if client tries to call add / remove…
Below are the sample classes I have created. I would like to get some thoughts on how I implemented.
- Am I implementing composite correctly?
- Is this a good piece of sample code?
- Thoughts on throwing an exception in leaf class for un-needed function implementations like add / remove Person?
Client class
public class Client {
public static void main(String args[]) {
Person gretchen = new Parent("gretchen", 101);
Person steve = new Parent("Steve", 57);
Person mary = new Parent("Mary", 56);
Person timmy = new Child("Timmy", 10);
Person molly = new Child("Molly", 5);
gretchen.add(steve);
gretchen.add(mary);
steve.add(timmy);
steve.add(molly);
mary.add(timmy);
mary.add(molly);
//introduces entire family
gretchen.introduce();
}
}
Component Class
public interface Person {
public String getName();
public int getAge();
public int getNumOfChildren();
public void add(Person person);
public void remove(Person person);
public void introduce();
}
Composite class
public class Parent implements Person {
private String name;
private int age;
private List<Person> children = new ArrayList<Person>();
public Parent(String name, int age) {
this.name = name;
this.age = age;
}
public void introduce() {
System.out.println("Hi, I am ".concat(getName()));
System.out.println("I am ".concat(Integer.toString(getAge()).concat(
" years old")));
System.out.println("I have ".concat(Integer.toString(children.size()).concat(" kids.")));
System.out.println("Children... introduce your selves!");
for (Person person : children) {
person.introduce();
}
}
@Override
public String getName() {
return this.name;
}
@Override
public int getAge() {
return this.age;
}
@Override
public int getNumOfChildren() {
return this.children.size();
}
@Override
public void add(Person person) {
children.add(person);
}
@Override
public void remove(Person person) {
children.remove(person);
}
}
Leaf class
public class Child implements Person {
private String name;
private int age;
public Child(String name, int age) {
this.name = name;
this.age = age;
}
@Override
public void introduce() {
System.out.println("Hi, I am ".concat(getName()));
System.out.println("I am ".concat(Integer.toString(getAge()).concat(
" years old")));
}
@Override
public String getName() {
return this.name;
}
@Override
public int getAge() {
return this.age;
}
@Override
public int getNumOfChildren() {
return 0;
}
@Override
public void add(Person person) {
// no implementation here, throw exception maybe?
}
@Override
public void remove(Person person) {
// no implementation here, throw exception maybe?
}
}
Solution
- Am I implementing composite correctly.
Although you are doing it correctly, a part of me wonders if it was the correct solution for you problem. The reason I ask is because a interface usually means that each method that you define is going to need to be implemented differently for each sub class that implements it. In your case it doesn’t matter what type of person you have they will always introduce themselves the same way, and they will always say their name the same way, and they will always tell their age the same way. That being said an abstract class would have been a better choice because the basics will be the same for both parent and child (and even if you decide to add another branch of a person) for things such as their name, and age. The introduction would possibly be the only exception if you added a culture to this mixture. Since different cultures introduce themselves differently in almost all scenarios that would be a good candidate for a interface.
- Is this a good piece of sample code?
The code appears to be in good order. It is easy to read and easy to understand. My only issue (and it is debatable) is having the age part of the constructor. In this context it makes sense, but when i first introduce my self to someone I don’t say, “Hi my name is Robert Snyder, I’m 30 years old”. Instead I just introduce my self as “Robert” but judging by the context of your code it might be fair to have the age (such as in the case of the entrance to a liquor store, they want your id which has your name and age)
- Thoughts on throwing an exception in leaf class for un-needed function implementations like add / remove Person?
No. Never. Only in the higher levels do you want that to happen. This forces the user to use child classes. If your child classes are “dangerous” then people will either make their own, or not use yours.
Thoughts on throwing an exception in leaf class for un-needed function implementations like add / remove Person?
In Java7, the Collections library will throw an UnsupportedOperationException if you call a method that changes the state of a collection returned by Collections.unmodifiableCollection. ImmutableList, from the Google Collections Library, will throw UnsupportedOperationException if you attempt to modify the state of the list.
So if you were already starting with an interface that allowed modification (Person), then this would be a reasonable way to deal with the leaf implementation (Child).
In designing a new interface, I’d try to stay away from that. There are a couple possibilities. One is that your leaf nodes don’t really have different behavior than the interior nodes – Child is just a Parent in the state where Parent.children.isEmpty(). In other words, the instances are all just nodes in a directed graph, some of which are end points. In that case, I’d just use a single class, and give the children empty arrays.
Note: with the Collections libraries, you get a free punt here, if you want it:
private List<Person> children = Collections.EMPTY_LIST;
And then let the empty list throw what ever it is going to throw when somebody tries to insert a new Person into it.
Another possibility is that Parent and Child really are two different ideas, but after the composite is constructed, those details are hidden from the consumer. In that case, you want a single interface that does NOT include the add and remove methods. Instead, you build a List first, and then pass that list to the Parent constructor. Once the construction is done, the composite nature is no longer important, so consumers only see the Person interface of the outermost Parent.
Another possibility is that the relationships between Person instances aren’t really part of the objects themselves, but belong to some other thing. It is fairly common in graph implementations to discover that there are Nodes, and Edges that connect Nodes. The logic that does the traversal knows about both, but the Nodes don’t know about their edges at all. Notice that in your implementation, you need to bake ordering into the objects themselves (breadth first? depth first? what order do you visit the children?). In that case, split the edges out from Person, and work with an abstraction (Family? SocialNetwork?) that manages the relationships when new Persons are added.
My most common cases are those where the nodes and edges are known during construction, and fixed from that point forward. So there, I use my concrete leaf and container classes to build the correct object graph from the outset, but I do not publish the availability of methods to change the contents of the graph. In other words, it only looks like a composite during the construction phase.
Having a so-called fat base class is not a good design. Functionality to manage children should be moved to the Parent class.
Also, it makes sense to make Person an abstract class to avoid redundancy, all persons having name and age. Keep in mind that not every person without children is a child.