Create a new object class or write a method which converts sub-class objects? or something else? Performance NOT Preference

Question

I've put together two separate programs which play a card game called 'Crazy Eights'.

The classes I've written for this program are based on a default 'card' package which provides playing card objects and some generic methods for playing cards.

I've taken two separate approaches to achieve this which are both functional in their own right.

Here are two UML class diagrams which depict the two approaches:

Inherited subclass 'conversion' method

Composed subclass with similar methods

As you can see in approach 1 the class EightsCard contains a method convert(Card) Here's the method:

  /**
   * Converts a Card into an EightsCard
   * @param card The card to be converted
   * @return The converted EightsCard
   */
   public EightsCard convert(Card card) {
     if (card != null) {
     EightsCard result = new EightsCard(card.getRank(), card.getSuit());
     return result;
     } 
     return null;
   }
 }

This method allows you to call methods from CardCollection which otherwise wouldn't be legal. For example, in the play method from the EightsPlayer class shown below:

  /**
   * Removes and returns a legal card from the player's hand.
   */
  public EightsCard play(Eights eights, EightsCard prev) {
    EightsCard ecard = new EightsCard(0, 0);
    ecard = ecard.convert(searchForMatch(prev));
    if (ecard == null) {
      ecard = drawForMatch(eights, prev);
      return ecard;
    }
    return ecard;
  }

Approach 2 doesn't require any conversions as the similar methods have been written in a new class EightsCardCollection which extends CardCollection. Now the play methods can be written like this:

  public EightsCard play(Eights eights, EightsCard prev) {
    EightsCard card = searchForMatch(prev);
    if (card == null) {
      card = drawForMatch(eights, prev);
    }
    return card;
  }

This brings me to a couple of questions:

• Are there any advantages to either approach beyond personal preference?
• Is there a better way to compose this program?

For example, might it be better to write 'similar' classes which are more specific¹ and not use default classes² at all.

¹ labelled 'crazyeights.syd.jjj' or 'chaptwelvetofort' in the class diagrams.

² labelled 'defaults.syd.jjj' or cards.syd.jjj' in the class diagrams.

Answer 1

Too many subclasses

Neither of these approaches is very good, as both of them have more subclasses than necessary. Why does EightsCard extend Card? What if you wanted to implement some other card game? (There are quite a few, you know...) Would you make one subclass for each card game? Please don't.

I would have these classes:

• Card
• CardCollection
• Player

I wouldn't even have a Deck class as it seems like it does nothing else than extend CardCollection without providing any more functionality.

Then I would have one class for each game implementation, so one EightsController class that handles the game logic for that game. No specific class for EightsCard, no specific class for EightsCardCollection, etc.

The reason is simple: You don't need anything more than this. A Card and a CardCollection is exactly the same no matter which card game you are playing. A Player is also the same thing in all games.

Answer 2

With regard to those domain models, I agree with Simon Forsberg that those are way too complicated, and I understand they're designed to demonstrate some concepts, but even then they could have used a more realistic example, like components in an automobile and their relations. You probably need only two domain classes, Card and Player. For an ordered collection of Cards, use a List<Card>. When it comes to dealing, taking turns, etc. there is a lot of room for creativity for how to organize game logic. Generally it is preferable to compose types out of other ones rather than rely heavily on inheritance which is less flexible in practice.

When it comes to performance, common rules apply. For instance, reuse objects where possible. If you're always using the 52-card deck then there's no reason to even have a Card class. There's probably nothing better-performing than reusing enumeration values everywhere.

enum Card {
    ACE_CLUBS,
    TWO_CLUBS,
    // ...
    ACE_DIAMONDS,
    // ...
}

(You can't really make this enum any more complicated, since different games use different orderings, place different values on different cards depending on the context, etc.)

For an excellent practical guide to programming for performance, I recommend the book, Java Performance: The Definitive Guide.

Answer 3

Simon's comment caught my attention: "I would prefer having two enums - one for Suit and one for Rank - instead of one enum for the whole card"

Is there a better way to compose this program?

The key point is this -- most of your program should be completely insulated from the in memory representation of a card.

In memory, a card might be

• an integer
• a byte
• an enum
• a String
• a URL (http://example.org/cards/hearts/7)
• an integer/byte restricted to the range [0,52)
• a tuple
  • int, int
  • enum, enum
  • byte, byte
  • mix and match
• a reference to a third party implementation

Most of your code should not care.

Ideally, the only bits that care at all would be the module that provides the implementation, and perhaps the composition root.

Most places in your diagram where you have <<Java Class>> you should instead have <<Java Interface>>.

See Parnas, 1972

In that sort of a design, you would use composition to attach your crazy eight semantics to a generic definition of card defined somewhere else.

Eights.Deck deck(Iterable<Generic.Card> standardDeck) {
    List<Eights.Card> cards = new ArrayList();
    for(Generic.Card c : standardDeck) {
        cards.add(
            new Eights.Card(c)
        );
    }
    return new Eights.Deck(cards);
}

or perhaps

Eights.Deck deck(Generic.Deck standardDeck) {
    return new Eights.Deck(
         standardDeck
             .stream()
             .map(Eights.Deck::new)
             .collect(Collectors.toList())
    );
}

The composition root would look something like

public static void main(String [] args) {
    GenericCards genericCards = new ApacheGenericCards();
    EightsCards eightsCards = MyAwesomEightsCardsV2(genericCards);
    EightsGame game = new EightsGame(eightsCards)
    game.play();
}