I came across a snippet like this, and find it’s over engineering. Is it a good practice?
public class SchoolList extends ArrayList<School> {
}
public class School extends ArrayList<StudentList> {
}
public class StudentList extends ArrayList<Student> {
}
public class Student {
}
Actually I prefer a straightforward option:
List<List<List<Student>>> schoolList;
2
Usually, you just use the existing JDK API. Why? Let me give you some feedback on your code:
public class School extends ArrayList<StudentList> {
}
or, in other words: a schools IS-A list of list of students(list of groups/classes?). What really is a school?
A school is an institution. You can find students inside(HAS-A), rooms, proffesors and more. It is obvious that if you want to redesign the above piece of code, you will end up writing:
public class School {
private List<Student> students; // or list of list? your choice
}
Also, if you look at the public methods, you will have an addAll
method, with Collection<StudentList>
as argument. A School
IS-A list of StudentList
(your implementation), so you can do this:
School s1 = ...;
School s2 = ...;
s1.addAll(s2);
Do you find the above code logical? I simply cannot imagine a School
adding another School
🙂 An addStudents
method seems more appropriate, like this:
s1.addStudents(s2.getStudents());
This line speaks for itself: students will go to another school next year 🙂
You should extend/implement the current classes/interfaces only when you feel it doesn’t suit for you. Imagine a circular list, the iterator should move to the first element at the end of the list. I don’t know any circular list implementations in the JDK API, so I am forced to implement my own, I have no other choice. You do.
It is a good practice to replace something like List<List<Map<String, Integer>>>
with a domain specific class. But in almost all the cases the best way to do is by aggregation not inheritance. This has some advantages:
- no tight coupling with the superclass
- you can choose what methods to expose; you may not want to use all the methods from ArrayList
- you do not violate encapsulation
In general it is bad to extend collections; if you want to have List
functionality then you should still use aggregation for functionality and implement the List
interface. For more details about this see Item 16 in Effective Java 2nd edition by Joshua Bloch.
In your case, instead of
public class School extends ArrayList<Student> {
}
use
public class School {
private List<Student> list;
}
or if you need List
functionality:
public class SchoolList implements List<Student> {
private List<Student> list;
}
2
Extending generic containers usually doesn’t make sense for anything which represents a business object.
public class School extends ArrayList<Student>
In this example, a school is an ArrayList
of Student
s. But is that everything a school is? What do you do when business requirements meant the school gets more attributes, like an ArrayList<Teacher>
or an ArrayList<ClassRoom>
?
It’s much better when classes don’t extend the container which represents their data. Instead of that, classes should be composites containing the container(s) which contain their data:
public class School {
private List students;
private List teachers;
private List rooms;
// methods for accessing and manipulating the above lists
}
But are there cases where extending collections makes sense? Yes, it does, when you want a more advanced container. There was one time where I needed a map which was able to tell which changes were made to it. So I created a public class LoggingHashMap extends HashMap<Object, Object>
which was overriding all mutator methods to also create an entry in a private journal and also had a method for getting and resetting that journal.
It might be good practice, but only if there is something the new type can do that the Generic type couldn’t.
Without context, it looks very much as if someone learned a construction that was necessary before generic types were invented and now keeps using it although it’s no longer needed. After all, type-safe collection access is pretty much why Generics were invented in the first place!
But there might be corner cases and special uses where the extends
actually makes sense. For instance, it can be tricky to pass a generically-typed variable to a method that expects vararg parameters. If that problem actually occurs, then having a self-defined type for common objects in your domain model might yield a benefit in readability that outweighs the extra definitions.
It depends. If you want to attribute domain behaviour to such collections, or if not all methods of an ordinary List would make sense on that collection, then I would see it as a good practice.
If anything, it communicates intent really well, but if that’s the only reason to do it, then you could argue it’s over engineered.