2
\$\begingroup\$

So I wrote a card game in Java/JavaFX but I don't particularly like this solution and I was wondering if you guys have any better solutions:

public static CardNode get(Card card, double nodeWidth, double nodeHeight, double imageWidth, double imageHeight) {
    if (card.isMonster()) {
        MonsterCardNode cardNode = new MonsterCardNode(nodeWidth, nodeHeight, imageWidth, imageHeight);
        cardNode.setCard(card);
        return cardNode;
    } else if (card.isSpell()) {
        SpellCardNode cardNode = new SpellCardNode(nodeWidth, nodeHeight, imageWidth, imageHeight);
        cardNode.setCard(card);
        return cardNode;
    } else if (card.isTrap()) {
        TrapCardNode cardNode = new TrapCardNode(nodeWidth, nodeHeight, imageWidth, imageHeight);
        cardNode.setCard(card);
        return cardNode;
    }
    return null;
}

CardNode and Card are abstract classes. They both implement interfaces Monster, Spell and Trap. Methods isMonster, isSpell, isTrap return a boolean and are essentially instanceof and flag checks. I was wondering if there is any better way to handle this with polymorphism.


Here is the usage of that static method.

model.getPlayer().getHand().addListener((ListChangeListener.Change<? extends Card> change) -> {
    while (change.next()) {
        if (change.wasAdded()) {
            change.getAddedSubList().forEach((card) -> {
            Platform.runLater(() -> {
                    CardNode cardNode = CardNode.get(card, 105, 145, 96, 105);
                    view.getPlayerView().getHand().getChildren().add(cardNode);
                });
            });
        }
    }
});
\$\endgroup\$

closed as off-topic by πάντα ῥεῖ, Stephen Rauch, IEatBagels, Dannnno, hjpotter92 May 16 '18 at 22:53

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – πάντα ῥεῖ, Stephen Rauch, IEatBagels, Dannnno, hjpotter92
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ You should add your cards implementation \$\endgroup\$ – IEatBagels May 16 '18 at 20:24
2
\$\begingroup\$

As a general principle, instead of asking an object what its type is, tell it what you want to do, and let overridden methods handle the differences between types:

return card.getCardNode(nodeWidth, nodeHeight, imageWidth, imageHeight);

and add a getCardNode abstract method to Card, so that each Card subclass can implement its own piece of the get logic.

\$\endgroup\$
  • \$\begingroup\$ It also removes the need to track these instance checks everywhere, which would even result in blowing up the application if you forgot. \$\endgroup\$ – Koekje May 16 '18 at 20:17

Not the answer you're looking for? Browse other questions tagged or ask your own question.