CODEWITHSUNDEEP
javaoopdesign-patternspolymorphism
nafkhanzam
nafkhanzam

Reputation: 337

How to avoid instanceof from this kind of case?

My current code in Player.java is like this:

    public boolean canPutCard(Card card) {
        if (!handCards.contains(card)) {
            return false;
        }
        if (card instanceof ActivableCard) {
            boolean hasEmpty = false;
            int i = card instanceof CharacterCard ? 0 : 1;
            for (int j = 0; j < fieldCards[i].length; ++j) {
                if (fieldCards[i][j] == null) {
                    hasEmpty = true;
                }
            }
            if (!hasEmpty) {
                return false;
            }
        }
        return (card instanceof LandCard && !hasPutLandCard) || (card instanceof PoweredCard
                && ((PoweredCard) card).getPowerNeeded() <= currentElementValue[card.getElementType().ordinal()]);
    }

I want to avoid using of multiple instanceofs here because of what they call it "code smells" and use polymorphism instead. So what I can do is to rewrite it to be like this:

    public boolean canPutCard(LandCard card) {
        return !hasPutLandCard;
    }

    public boolean canPutCard(PoweredCard card) {
        return card.getPowerNeeded() <= currentElementValue[card.getElementType().ordinal()];
    }

    private boolean canPutCard(int i, PutableCard card) {
        boolean hasEmpty = false;
        for (int j = 0; j < fieldCards[i].length; ++j) {
            if (fieldCards[i][j] == null) {
                hasEmpty = true;
            }
        }
        return hasEmpty;
    }

    public boolean canPutCard(PutableCard card) {
        return canPutCard(1, card);
    }

    public boolean canPutCard(CharacterCard card) {
        return canPutCard(0, card);
    }

    public boolean canPutCard(Card card) {
        return handCards.contains(card);
    }

classes:

abstract class SkillCard extends Card implements PoweredCard
class CharacterCard extends Card implements PoweredCard, PutableCard
class LandCard extends Card

But, surely it won't work because it doesn't check every cases..

Is it possible to avoid instanceof and use polymorphism instead in this case?

Upvotes: 0

Views: 110

Answers (1)

suff
suff

Reputation: 66

Instanceof operator can usually be replaced with proper reliance on java virtual dispatch. Move decision logic to Card class, like this 

public abstract boolean canPutCard(Player player);

And then implement appropriate logic in Card subclasses like:

class LandCard extends Card {
    @Override public boolean canPutCard(Player player) {
         return !player.hasPutLandCard() && player.handContains(this);
    }
}

class CharacterCard extends Card implements PoweredCard, PutableCard {
    @Override public boolean canPutCard(Player player) {
         return !player.hasEmptyCharacterCardField() && player.handContains(this);
    }
}

... and so on. Player class will need to have required query methods.

Upvotes: 1

Related Questions