How to make this method more generic

Question

i am having some trouble figuring something regarding inheritance in Java. I thought it would be straightforward but it has stumped me.

I have this superclass..

public class MyItem {

    private String barCode;
    private String price;

    public String getBarCode() {
        return barCode;
    }

    public void setBarCode(String barCode) {
        this.barCode = barCode;
    }

    public String getPrice() {
        return price;
    }

    public void setPrice(String price) {
        this.price = price;
    }

}

And i have these 2 subclasses

public class PromotionalItem extends MyItem {

    private String promotion;

    public String setPromotion(String promotion) {
        this.promotion = promotion;
    }

    public void getPromotion() {
        this.promotion = promotion;
    }

}

public class SellableItem extends MyItem {

    private String quantity;

    public String setQuantity(String quantity) {
        this.quantity = quantity;
    }

    public void getQuantity() {
        this.quantity = quantity;
    }

}

Now i have a method that i want to make generic, i thought something like this could work...

public void processItem(MyItem item){
    if(item.getClass().isAssignableFrom(PromotionalItem.class)){
        processPromotionalItem((PromotionalItem)item);
    }
    else{
        processSellableItem((SellableItem)item);
    }
}

But I am getting a ClassCastException when i try to cast these items as their respective subclasses. I thought something like this would be do-able. Am i missing something? What is the alternative do something like this?

Answer 1

Make MyClass abstract and add an abstract process method, like @JoakimDanielson suggested. Then, in your child classes, override that method and implement your own logic.

public abstract class MyItem {
    ...
    public abstract void process();
}

public class PromotionalItem extends MyItem {
    ...
    @Override
    public void process() {
        // do whatever
    }
}

 public class SellableItem extends MyItem {
    ...
    @Override
    public void process() {
        // do whatever
    }
}

Then, in your processItem method, just call process:

public void processItem(MyItem item) {
    item.process();
}

Answer 2

The code looks like an anti-pattern. What I would do is have an abstract method called process in MyItem and have both subclasses implementing that method:

public class MyItem {

    private String barCode;
    private String price;

    public String getBarCode() {
        return barCode;
    }

    public void setBarCode(String barCode) {
        this.barCode = barCode;
    }

    public String getPrice() {
        return price;
    }

    public void setPrice(String price) {
        this.price = price;
    }

    public abstract void process();

}

Now if you have a subclass you are forced to implement the process method, and then instead of checking what class it is you can just call the process method directly.

public void processItem(MyItem item){
    item.process();
}

Answer 3

Use the instanceof keyword

if(item instanceof PromotionalItem){
    processPromotionalItem((PromotionalItem) item);
} else if(item instanceof SellableItem) {
    processSellableItem((SellableItem) item);
}

Make sure you use else if not only else because item might be something else other than PromotionalItem and SellableItem if you cast it to a class from which it wasn't build from, you will get a ClassCastException

instanceof is a keyword that is used for checking if a reference variable is containing a given type of object reference or not.

Answer 4

In your case you should use instanceof instead of isAssignableFrom (be careful though, the syntax is different, more on that below).

isAssignableFrom checks if the parameter object can be written to the object the function has been called from. instanceof checks if the left object is from the same class or a subclass of the right class. This will make more sense once you've seen the syntax of instanceof:

if(item instanceof PromotionalItem){
    processPromotionalItem((PromotionalItem)item);
}

So in a nutshell, your logic was just a little off. You were trying to cast from one subclass of your item class to a completely different subclass.

Answer 5

I‘ll only address the issue, as there are enough solutions (I think @Major Ben s one is nice)

item.getClass().isAssignableFrom(PromotionalItem.class)

What this line means is: „Can I assign to the dynamic class of item an instance of PromotionalItem.“

But now consider this - is it legal?

MyItem item = new PromotionalItem();

Yes, it is. So this will always be true. Hence, you then try to cast to PromotionalItem, even when it is actually not ok.

Also have a look at this. https://stackoverflow.com/a/3657960/2995907

Answer 6

Using abstract class which is great. We can think also with generices like

public class MyItem<T extends MyItem> {

    private String barCode;
    private String price;

    public String getBarCode() { return barCode; }

    public void setBarCode(String barCode) { this.barCode = barCode; }

    public String getPrice() { return price; }

    public void setPrice(String price) { this.price = price; }

    public void process(T item) {
        if(item instanceof PromotionalItem){
            System.out.println("PromotionalItem");
            //do something for promotionalItem
        } else if(item instanceof SellableItem) {
            System.out.println("SellableItem");
            //do something for SellableItem
        }
    }
}


public class PromotionalItem extends MyItem {

    private String promotion;

    public void setPromotion(String promotion) {
        this.promotion = promotion;
    }

    public String getPromotion() {
        return promotion;
    }
}


public class SellableItem extends MyItem {

    private String quantity;

    public void setQuantity(String quantity) {
        this.quantity = quantity;
    }

    public String getQuantity() {
        return quantity;
    }

}


    @Test
    public void test_porecessItem() {
        PromotionalItem promotionalItem = new PromotionalItem();
        SellableItem sellableItem = new SellableItem();
        MyItem<PromotionalItem> promotionalItemMyItem = new MyItem<>();
        MyItem<SellableItem> sellableItemMyItem = new MyItem<>();
        promotionalItem.process(promotionalItem);
        sellableItemMyItem.process(sellableItem);
    }

By the way, this is just an option which we can think.