CODEWITHSUNDEEP
javainstanceof
javaNoober
javaNoober

Reputation: 1338

Java: Using instanceof to expose different object methods

I've read this article http://www.javapractices.com/topic/TopicAction.do?Id=31, which says using instanceof to check what my object is beforehand is a bad practice.

But I don't know how to do it otherwise in this case: I have an API that returns a List of Items. This list of Items returns both Users and Admins

If I cast an item like (User)item, I'll have a ClassCastException as soon as it hits an Admin

for(Item item : items){                     
                if (item instanceof User){
                    ((User)item).getName());
                    ((User)item).getEmail());
                }
                else if (item instanceof Admin){
                    ((Admin)item).getName());
                    ((Admin)item).getEmailList().getPrimary());
                }                           
            }

Since it is an API I can't modify Item, Item do not have the methods on the childs, and the email retrieve method is different. Do I have another choice?

Upvotes: 1

Views: 1164

Answers (3)

MRalwasser
MRalwasser

Reputation: 15953

Define an interface with all common methods, let User and Admin classes implementat that and use only that interface for accessing your objects in these cases.

EDIT (because you cannot change the API)

The alternative options are:

  1. Stick to your current solution. This seems to be the best trade.

  2. Define an interface, create two wrapper classes which implement this interface and wrap the original objects. From an OOP perspective this is more clean, but I think this is a little bit oversized in your case, but it really depends.

  3. Theoretical(!): you could also use reflection, but don't do this - it would make the code even more worse - especially not when there are only two different compile-time known Item subtypes

Upvotes: 5

Andy
Andy

Reputation: 9048

Ideally you would change the interface to Item so that it exposed methods such that it didn't matter if you were dealing with a User or an Admin in your code -- so that Item defines both getName() and getEmail() and User and Admin implement these methods as appropriate.

However, since you don't control the API you are using then you have little choice but to write your code as you have it now.

So, the point made by the referenced article is a good one, but it assumes that you can change the classes you are working with.

Upvotes: 2

AlexR
AlexR

Reputation: 115328

First, your code does not have any effect. You are calling getters without any assignment, so you actually do not use the getter's return value.

Second, it is a bad practice to have collection with elements of different types. In your ccase User and Admin.

Third, if you even need such collection you can use other patterns to perform what you need. For example Visitor pattern looks like a good alternative to casting.

Upvotes: -1

Related Questions