Java using instanceof operator for this example

Question

I am developing a BlackJack game using Java, and it came to a point that I am using instanceof operator to determine if it is a type of some subclass.

Here's an example.

public void checkForBlackJack(Player player) {
        Hand fHand = player.getHands().get(0);
        if (fHand.getCardScore() == 21) {
            fHand.setBlackjack(true);
        }

        if(player instanceof BlackJackPlayer){
            BlackJackPlayer bjplayer = (BlackJackPlayer) player;

            if(bjplayer.isSplit()){
                //Check the users other hand if it is already blackjacked
            }
        }
    }

Is using instanceof considered to be bad for such example? and just an overview here is my class diagram

I decided to use instanceof to check for blackjack , is because The Dealer can also blackjack.

Answer 1

In this case you could push the isSplit method to the Player class and implement it with return false;. Then the need to instanceof disappears.

I wouldn't say that it is bad to use instanceof but often there are other options that are better.

Answer 2

why don't you declare a function for both types of Player?

public void checkForBlackJack(BlackJackPlayer blackjackplayer) {
  // ....
 }

public void checkForBlackJack(BlackJackDealer blackjackdealer) {
  // ....
 }

Answer 3

In your case it seems like the checkForBlackJack-method lives in the wrong place (I am guessing a class containing game logic). If you move [1] that method to the Player class you can then get rid of the conditional logic by breaking it out to an abstract method. This is commonly referred to as "replace-conditional-with-polymorphism" [2]. In case you have common behaviour in the two subclasses, you can put that behaviour in the base class instead of making that one abstract (so you wont have to repeat yourself).

[1] http://sourcemaking.com/refactoring/move-method

[2] http://sourcemaking.com/refactoring/replace-conditional-with-polymorphism

BR Sebastian

Answer 4

As a first pass at a refactor, I would structure your code like this:

public class Player {
    public boolean hasBlackJack() {
        // Handle stuff outside of your "if instanceof" above
    }
}

public class BlackJackDealer extends Player {
    // Other methods, not overriding hasBlackJack method
}

public class BlackJackPlayer extends Player {
    @Override
    public boolean hasBlackJack() {
        super();
        // Handle stuff inside your  "if instanceof" above
    }
}

Though this does get rid of the instanceof, the real reason I prefer it is a little deeper. Who is responsible for determining if a Player has a blackjack? The player of course! This is shown in your code when you're doing something like:

Hand fHand = player.getHands().get(0);

Woah; why is this Object reaching inside the Player object to get his hands out of there? Instead, ask the player object whether he has a blackjack, and proceed accordingly. Notice you can still have a "GameRunner" class responsible for deciding what to do next if the player has a blackjack (payout appropriately, remember betsize, etc), but checking if there is a blackjack is a question you can ask the Player object.

Going a little bit further, this code seems dodgy to me:

    if (fHand.getCardScore() == 21) {
        fHand.setBlackjack(true);
    }

An external entity shouldn't be telling the Hand that it is a blackjack and then mutating the Hand. Is there a situation where 21 is not a blackjack? If not, you can remove this call entirely and just use getCardScore() == 21 to determine if it's a blackjack. If there is, then you can write the method isBlackjack() in the Hand class itself.