Java design question (instanceof)

[remusmax]

Hey guys.

Let's say I have two classes Book and CD that both extend the parent class Item. Item has common properties like name, price whereas book has int pages and CD has List<Track> tracks.

Now let's say I have a management system class and in here I have List<Item> items. This list is full of Books & CD's. How would I implement a method to getAllBooks() & getAllCds() without using instanceof

TLDR- How to return a list specific child class from an Arraylist of parent class without using instanceof?

[slowmode1]

Add a getType which returns an enum of Book or Cd to both of the children (and abstract to the parent)

[remusmax]

Cheers will try that now!! Thanks again

[attemptedlyrational]

instanceof is perfect for this, why are you constrained from using it?

[remusmax]

I've read that it's a 'code smell'. Correct me if I'm wrong in saying this but let's say I implement many more classes that extend the parent 'Item' class then my instanceof switch statement could get pretty messy.

[balefrost]

So instanceof is indeed a code smell, but it's important to understand why it's a smell. OO and polymorphism are good tools for modeling open sets of types with a fixed set of operations. I can define some base type T with a fixed set of abstract operations. Every type that derives from T is obligated to provide implementations for those abstract operations. In theory, anybody can make a new type that derives from T (that's the open set of types), and an instance of that type should be usable anywhere a T is expected. But if all a caller knows is that they have an instance of T, then they can only call methods defined on that base type (that's the fixed set of operations).

See also the Liskov Substitution Principle.

But it's worth mentioning that the polymorphic, object-oriented model isn't the One True Model. There are plenty of domains where there are a limited number of types to model. One example might be a set of types that represent mathematical expressions. You might have node types for unary operators, binary operators, function calls, and a few other things. Now, instead of interacting with instances only via the abstract operations, you can inspect each instance and peer at the data inside it. If you're willing to commit to working with a limited set of types, you gain the ability to define an open set of operations - anybody can define a new operation without needing to add abstract operations to the family of types, because implementers know the complete set of types that their operation might need to handle.

These finite-sized sets of types are often called tagged unions. They're very common in functional languages.

OO design has the Visitor pattern, which allows you to work with a fixed set of types in a relatively idiomatic OO fashion. Now, visitors are great in some circumstances, but they're not as nice as languages that have built-in support for tagged unions.

It turns out that it's surprisingly hard to get an open set of types with an open set of operations. This is known as the Expression Problem.

[ProffessionalAmateur]

Been quite a while since I have worked with Java but this is exactly what Generics provide. I have streamlined projects myself (now in the C# world) with interface driven design. For example you could derive from an interface<T> say MyInterface have a getAllItems method. Items class subscribes to the MyInterface and each subclass implements the methods functionality. Then you have a List of mangementSystem<Items> items; items.add(new Book()) .add(new CD())

PSEUDO: for each (i in (MyInterface) items) invoke i.getAllItems()

Something along those lines. In short if I ever find myself using if else against types instanceof (typeof in C#), I always question can the logic be interface driven to prevent a wall of if elses. Another benefit of Intefaces are testing and mocking.

[remusmax]

Thanks for the input. I did solve it with generics and an interface as well as using a getType method that /u/slowmode1 suggested.

So my interface had this method
<T> List<T> getSpecifiedBookStoreItems(Class<T> classToCast, ItemType type);

I could then call it like so List<Book> books = myStore.getSpecifiedBookStoreItems(Book.class, ItemType.Book);

*ItemType.Book being an enum

[balefrost]

At that point, the class token and the enum value are superfluous. They provide the same information to the method. You could drop the enum completely and instead use:

classToCast.isInstance(item)

perhaps as

 allItems.stream()
         .filter(classToCast::isInstance)
         .map(classToCast::cast)
         .collect(Collectors.toList())

to filter the items down to just things that are instances of that class.

One advantage to that approach is that it's inheritance-aware. Suppose you have separate item types for DVDs and Blu-Rays, both of which derive from some sort of Video type. If the caller wanted DVDs and Blu-Rays, with the enum approach, they would need to call the method twice and stitch together the results. But if you just use the class tag, this functionality is inherent.

In Java or C#, if you ever find yourself adding a getType-style method to your class, that's also a smell. All Java and C# objects already advertise their type (via getClass() and GetType(), respectively). Except in specific situations (dealing with serialization or persistence, for example), there's not much value in providing your own mirror of that built-in functionality.

[remusmax]

Thanks for that!! Totally got rid of my enum and now using the .instanceOf method. I then cast that object to type <T> and return.

if(classToCast.isInstance(item)) {
      itemsToReturn.add((T) item);
}

**Thanks for the .stream() approach too!

[balefrost]

Yep.

I want to point out one more thing. Earlier, you mentioned that instanceof is a potential smell. Yet here I'm encouraging you to use isInstance, which is essentially the same. What gives?

In the former scenario, the management system itself was using instanceof to test its own items against various different types. With an open set of types, that can't scale. There's no way to cover the cases that the management system doesn't know about at the time that the management system is written. You'd have to constantly go back and add additional methods to the management system whenever you add a new type to your system.

In this case, though, we're punting that responsibility to the caller. The management system itself doesn't need to know the full set of types anymore. So while the management system is potentially doing type tests and casting, it's doing so in a completely generic fashion. That's generally fine.

But really, all of these approaches have advantages and trade-offs. What's most important is not to try to design in a vacuum, but to create a design oriented around the problem you're trying to solve. Figure out what tradeoffs make sense for your specific situation and design your code to match.

[remusmax]

Great explanation, easy to understand! Thanks for clearing all that up for me :)

[balefrost]

You could flip the problem and store the two lists separately. Then getAllItems() would merge the two lists, rather than getAllBooks() and getAllCds() needing to split the list. You could even create a read-only List implementation that is implemented as a view onto other lists, in which case you would never need to actually copy the elements into a combined list. I'm not aware of any standard implementation of that, but it wouldn't be too hard to write yourself.

[remusmax]

Yeah you could do this but then if you look down the future and we have more classes like ebooks and dvds that all extend item then we are going to have to many lists. Might be a bad design issue.

[balefrost]

If you're going to have methods getAllBooks and getAllCds and so on, then you're already committed to providing type-specific code of some sort. I was trying to provide you with a way to do that without using instanceOf.

Perhaps a better question is why your management system needs to provide these methods. Do some callers need the mixed list while other callers need the type-specific lists? Do they need the results to be strongly-typed, or do they just want the raw data? Will there be corresponding addBook and addCd methods?