1

EDIT: This is not a duplicate because I am addressing on issue where the classes cannot be overloaded to achieve polymorphism. I am not asking for comparison between polymorphism vs instanceof. I am asking a very specific scenario-based question.

I have read it n times online that one should always try to avoid the usage of instanceof operator, unless we are using it to implement the .equals() method of a class.

It was said that, whenever possible, we should try to achieve polymorphism rather than using instanceof to determine the type of the objects to decide the following actions.

For example: http://www.javapractices.com/topic/TopicAction.do?Id=31

However, in this specific scenario where I do not have any available methods in the super & sub classes (other than the toString, equals, accessor & mutator methods). Is it a valid scenario to use instanceof ?

class Warrior
{

    int damage;
    int defense;
    Item handSlot;

    public Warrior(){
        damage = 0;
        defense = 0;
        handSlot = null;
    }

    private void equipHand(Item item)
    {
        //Determine class of object to decide further actions
        if (item instanceof Weapon){
            this.handSlot= item;
            this.damage += ((Weapon)item).getDamage();  
        }
        else if (item instanceof Shield){
            this.handSlot = item;
            this.defense += ((Shield)item).getProtection();
        }
    }
}


class Item
{
    private String name;
    public Item(String name)
    {
        this.name = name;
    }       
}

class Shield extends Item
{
    private int protection;

    public Shield(String name, int protection){
        super(name);
        this.protection = protection;
    }

    public int getProtection(){
        return protection;
    }
}

class Weapon extends Item
{
    private int damage;

    public Weapon(String name, int damage){
        super(name);
        this.damage = damage;   
    }

    public int getDamage(){
        return damage;
    }   
}

Note that, if Weapon, Item & Shield class have a overloaded method (for example: equip()), I can simply use polymorphism. But in this case, the equip method is in another class. In this specific scenario, is it okay to use instanceof operator? Or is it still a bad design?

user3437460
  • 17,253
  • 15
  • 58
  • 106
  • 1
    Why not overload `equipHand(Item item)` with `equipHand(Weapon weapon)` and `equipHand(Shield shield)` as they look like they should both be handled in a different way. – Oli Jan 16 '15 at 17:01
  • Your question itself has answer "avoid whenever possible", if avoid not possible there is nothing wrong in using "instanceof" – kosa Jan 16 '15 at 17:02
  • @Oil that can be done, however the codes I shown above has been heavily cut down by me before posting here. Imagine if there are 100 possible items, I may have to write 100 overloaded methods that way. I am looking for a more universal approach. – user3437460 Jan 16 '15 at 17:09
  • As you know `Shield` and `Weapon` both are extend `Item`. So better to override `equipHand` method in each classes with their function. Direct access method with item instance instead of checking with `instance of` – bNd Jan 16 '15 at 17:10
  • @bmthaker So in short, you agree with what Oil said ? But won't it be a bad design when I have to write so many additional overloaded methods for every additional item class I add in? – user3437460 Jan 16 '15 at 17:20
  • @user3437460 To avoid additional overloaded method, Create different interface as per requirement.i.e. IWeapon, iShield like wise.Define specific methods in mention interface. To more clear read first chapter of `Head First Design Pattern` book, this will help you a lot. – bNd Jan 16 '15 at 17:31
  • @bmthaker Thankyou for your reply, you may consider putting down your answer here? It would be helpful for future programmers who visit this page. – user3437460 Jan 16 '15 at 17:36
  • @user3437460 welcome. Noted!! – bNd Jan 16 '15 at 17:43

2 Answers2

3

An alternative to your design would be to move the equipHand method from Warrior to Item (perhaps renaming it in light of this as well):

/* in Item */
public abstract void giveToWarrior(Warrior w);

/* in Weapon */
@Override
public void giveToWarrior(Warrior w) {
    w.setHandSlot(this);
    w.increaseDamageBy(getDamage());
}

/* in Shield */
@Override
public void giveToWarrior(Warrior w) {
    w.setHandSlot(this);
    w.increaseProtectionBy(getProtection());
}

(I've used some methods that do not exist in Warrior, but their functions should be self-explanatory. Making the indicated change would require implementing these methods.)

This design resolves the major issue with your current design, namely that it will become cumbersome to add new Item subclasses since doing so would require updating equipHand as well.

arshajii
  • 127,459
  • 24
  • 238
  • 287
  • This is what I was trying to overcome, and you addressed directly to what I asked. Nice implementation. Already +1 anyway :) – user3437460 Jan 16 '15 at 17:45
1

As you know Shield and Weapon both are extend Item. So better to override equipHand method in each classes with their function. Direct access method with item instance instead of checking with instance of.

The issue with above implementation, it will overload additional method for each class.

To avoid additional overloaded method, Create different interface as per requirement.i.e. IWeapon, iShield like wise. Define specific methods in mention interface. To more clear read first chapter of Head First Design Pattern book, this will help you a lot.

bNd
  • 7,512
  • 7
  • 39
  • 72