1

I am working on an implementation of the card game UNO (mostly to run simulations to test some house rules, but that's a different story).

For those who've never played it, it's similar to Crazy Eights. Players take turns playing a card on the discard pile. The card has to match in number or color. There are also draw cards, which force the next player to draw either two or four cards. It's also quite friendly to house rules, which makes it pretty interesting.

I have a Card class with the value and color of the card. What I would like to do is to create DrawCard which extends Card. Then, in determining the game mechanics, I would use instanceof to test whether I have a DrawCard.

I know that instanceof is often problematic, but this seems to be okay to me. Usually the cards are processed the same, and only a few special cards are treated differently, and only in specialized circumstances specific to the card type (That seems like a slippery-slope thing to say though...)

I could just use markers in the Card class (actually, each type of card already has its own 'value'), but I am extending Card anyway in order to have some methods that other types of cards may have (it's not just a means of identification). Using instanceof seems more general to me, as I wouldn't need to know about which values of value require this special behaviour, and it would be easy, for example, to add a Draw 8 card to the current contents of Draw 2 and Draw 4. (Saying this makes me wonder if I could use some sort of nested enum or something, but I don't know)

I know that both solutions would work (make the program run). To me using instanceof feels nicer, but I don't have the experience to understand whether it is okay. Is this bad design?

Mar Johnson
  • 215
  • 1
  • 7
  • you my find the answers on this post helpful: [The performance impact of using instanceof in Java][1] [1]: http://stackoverflow.com/questions/103564/the-performance-impact-of-using-instanceof-in-java – Codejunky Jul 12 '14 at 04:22
  • Does the "draw" card (not the class) also have value **and** color? If not, then inheritance is not the proper solution here -- inheritance is a "is a" relationship. Note that, if the "draw" cards do have values or colors, they must have the exact same semantics for them to be considered. – acdcjunior Jul 12 '14 at 05:06
  • @acdcjunior Yes, draw cards have colors – Mar Johnson Jul 12 '14 at 05:19

2 Answers2

1

The short answer: Not really acceptable, so yes - it's a sign of a design deficiency. instanceof is almost always a smell except in rare cases where a library is trying to do some kind of assistance that is meta or otherwise ugly but helps overcome a language/environment shortcoming.

With that opinion, the question is sort of a duplicate. See the answer to:

When is it acceptable to use instanceof?

It suggests to use the visitor pattern. This may be true if you genuinely have two different types in the system.

You also don't have to use the visitor pattern 'explicitly' but rather incorporate it in to your program flow. So consider an interface card:

interface Card {
  void pick(Player player);
}

class DrawCard implements Card {
  void pick(Player player) {
    player.draw(value); // assume value passed in e.g. ctor
  }
}
Community
  • 1
  • 1
Doug Moscrop
  • 4,479
  • 3
  • 26
  • 46
  • OK This makes sense. The way my game is currently set up, the game control class passes the card to the player's play method. So I could put methods in the Card interface to determine properties of the card? Something like a "does the next player to see this card have to draw" boolean method? – Mar Johnson Jul 12 '14 at 05:14
  • That's fine, I would just have the player, in response to being given a card, then 'pick' the card (you can rename it to `onTaken` or something like that if it makes more sense), but then have the card implementations call methods on the player to continue the turn. This might mean adding points to the player (player.increaseScore) or telling the player they have to do more things (player.takeMoreCards). – Doug Moscrop Jul 12 '14 at 06:11
0

I would say in general it's best to avoid instanceof wherever possible.

While it's somewhat vague since you didn't post any actual code, one option in this type of situation would be to implement two different interfaces, say ICard and IDrawCard and have the Card class implement the ICard interface and the DrawCard class implement both interfaces.

Then you would be able to differentiate the DrawCard class by virtue of its interface in function signatures, etc. (that is, refer to it as an object of type IDrawCard).

Edit in response to comment from OP:

In terms of handling the action after the card is played, you can leverage the interfaces to override the function that handles the actions. Since that's the context for this, I would probably recommend three different interfaces:

  1. ICommonCard
  2. ICard
  3. IDrawCard

Where ICommonCard is the generic interface for when you need that which perhaps handles has some general-purpose methods, and the other two interfaces handle the needs of those two specific types.

Then say you have an action handler method named handleAction, which could be defined thusly:

void handleAction(ICard card)
{
    // do ICard stuff here
    return;
}

void handleAction(IDrawCard card)
{
    // do IDrawCard stuff here
    return;
}

And then when you call that function, you can pass off an object which is the type of the interface into the overloaded function (one option would be to have a member variable for each type, and if it's going to be one or the other, pass the one that isn't null into the function).

khampson
  • 14,700
  • 4
  • 41
  • 43
  • Thanks, I probably should use interfaces. How does this change using instanceof to identify them, though? – Mar Johnson Jul 12 '14 at 04:45
  • Well, it really depends on exactly what you're trying to do. As I said above, you can refer to them by their interface in function signatures, etc. so that can help to differentiate. What's the context, specifically? – khampson Jul 12 '14 at 04:54
  • The cards have different behaviours, like what happens after the card is played. I need to know what the type of the card is so I can determine what is a valid action after it is played given the last card played. – Mar Johnson Jul 12 '14 at 05:04