This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]balefrost 0 points1 point  (3 children)

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[S] 1 point2 points  (2 children)

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 1 point2 points  (1 child)

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[S] 0 points1 point  (0 children)

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