Java group objects by subtype

Question

I have a list of super classes (List<SuperClass> classes = ...) that I want to split into separate lists based on their subtypes. I want to end up with lists parameterized with the subtype, i.e. List<A> aClasses = ... not List<SuperClass> aClasses = ...

Given this class hierarchy:

abstract class SuperClass {...}
class A extends SuperClass {...}
class B extends SuperClass {...}
class C extends SuperClass {...}

Here's what I have so far, which does work. The issue is that when iterating over the aClasses, bClasses, and cClasses lists, I need to cast the object inside the loop.

List<SuperClass> classes = ... // mix of A, B, and C

Map<Class<? extends SuperClass>, List<SuperClass>> grouped = classes.stream()
    .collect(Collectors.groupingBy(SuperClass::getClass, Collectors.toList()));

List<SuperClass> aClasses = grouped.get(A.class);
List<SuperClass> bClasses = grouped.get(B.class);
List<SuperClass> cClasses = grouped.get(C.class);

I know this works, but I'm looking for more succinct code (probably using streams). Although after writing out the code below, I'm thinking anything more I can do with streams will probably be just about as "complicated" as the code below.

List<A> aClasses = new ArrayList<A>();
List<B> bClasses = new ArrayList<B>();
List<C> cClasses = new ArrayList<C>();
for (SuperClass superClass : classes) {
    if (superClass instanceof A) {
        aClasses.add(superClass);
    }
    else if (superClass instanceof B) {
        bClasses.add(superClass);
    }
    else if (superClass instanceof C) {
        cClasses.add(superClass);
    }
}

The resulting classes will be inputs into methods with these signatures:

void processA(List<A> aClasses);
void processB(List<B> bClasses);
void processC(List<C> cClasses);

And I'm not going to do this: processA((List<A>) (List) aClasses);

Streams won't help you here. You'll always have to do an instanceof and cast, and that's something you want to avoid, no matter which way you do it. Of course the whole premise is wrong, as you would never actually have a `Color` class and then individual color classes that extend it. Instead of streams, I suggest you concentrate on "prefer composition over inheritance". — Kayaman, Jan 23 '20 at 16:13
`Map, List> map;`. Then you have some `add` function which checks if the `List` is already created, if not creates and adds the `Color` to it, otherwise creates a `List`, adds the `Color` to it, and then puts it into the `map` via `map.put(color.getClass(), list);` (or use one of the many pre-created `MultiValueMap` implementations from (e.g.) spring, apache commons, etc.). You might need to do some casting if you really need them to be a `List`, etc. instead of `List`, but that's up to your needs. — BeUndead, Jan 23 '20 at 16:18
@Kayaman I'm not actually doing anything with colors in my real code... this was just an example. I just wanted to use something simple that could show inheritance. — Matt, Jan 23 '20 at 16:22
@Michael 2 lines, but it doesn't get me exactly what I want. — Matt, Jan 23 '20 at 16:26
@Matt and that's the problem with examples like these, you shouldn't be using inheritance here. Inheritance should be [avoided](https://stackoverflow.com/questions/49002/prefer-composition-over-inheritance), unfortunately it took a while to realise that and there's a lot of "woo inheritance is great" text around. — Kayaman, Jan 23 '20 at 16:28
@Matt what do you get as an output to `Map, List> groupedColors = colors.stream() .collect(Collectors.groupingBy(Color::getClass, Collectors.toList()));` ? and why does `List reds = groupedColors.get(Red.class).stream().map(Red.class::cast).collect(Collectors.toList());` further not work? — Naman, Jan 23 '20 at 16:29
There is actually little point in casting each individual element and creating a new list. It's a waste of time at run-time. In this case, it is actually safe to cast the entire list. It requires an ugly double cast, but it is safe `List reds = (List) (List) groupedColors.get(Red.class)` — Michael, Jan 23 '20 at 16:44
I'm not asking for what's right or wrong about this! I'm just looking for possibly a better way. It seems people are getting hung up on the fact that I used colors as my example. I guess I should've just used classes A, B, C, D. The resulting lists are inputs to methods that require lists using the subtypes. `processRed(List reds)` — Matt, Jan 23 '20 at 16:49
@Matt well my problem is that you're using inheritance and you're using it wrong. The whole idea of inheritance is that you don't need to care whether it's `Red` or `Blue`, you can call `color.paint()` and the subclass will do its thing. If you see or write this kind of code, it's a red flag, no matter how cleverly it's done. — Kayaman, Jan 23 '20 at 17:06

score 0 · Answer 1 · answered Jan 23 '20 at 16:24

I'm not aware of any better way than what you describe, but I'd like to see it if it exists.

The instanceof check might be the best route and is probably what I'd do. The Map route you described will require casting because the type is unknown.

You can do it with streams, but that would require iterating over the list three times. If the list is small, that might be okay, but I'd still reject this in a code review. That said, it might be useful if you need one color.

List<Red> reds = colors.stream()
            .filter(color -> color instanceof Red)
            .map(color -> (Red)color)
            .collect(Collectors.toList());

Still, I wouldn't use this beyond a single color. As soon as you add a second color, the standard for loop approach is better.

Java group objects by subtype

1 Answers1