16

Background

Here's the actual problem I'm working on: I want a way to represent cards in the card game Magic: The Gathering. Most cards in the game are normal-looking cards, but some of them are divided into two parts, each with its own name. Each half of these two-part cards is treated as a card itself. So for clarity, I'll use Card only to refer to something that's either a regular card, or half of a two-part card (in other words, something with only one name).

cards

So we have a base type, Card. The purpose of these objects is really just to hold properties of the card. They don't really do anything by themselves.

interface Card {
    String name();
    String text();
    // etc
}

There are two subclasses of Card, which I'm calling PartialCard (half of a two-part card) and WholeCard (a regular card). PartialCard has two additional methods: PartialCard otherPart() and boolean isFirstPart().

Representatives

If I have a deck, it should be composed of WholeCards, not Cards, as a Card could be a PartialCard, and that wouldn't make sense. So I want an object that represents a "physical card," that is, something that can represent one WholeCard, or two PartialCards. I'm tentatively calling this type Representative, and Card would have the method getRepresentative(). A Representative would provide almost no direct information on the card(s) it represents, it would only point to it/them. Now, my brilliant/crazy/dumb idea (you decide) is that WholeCard inherits from both Card and Representative. After all, they are cards that represent themselves! WholeCards could implement getRepresentative as return this;.

As for PartialCards, they don't represent themselves, but they have an external Representative that is not a Card, but provides methods for accessing the two PartialCards.

I think this type hierarchy makes sense, but it's complicated. If we think of Cards as "conceptual cards" and Representatives as "physical cards," well, most cards are both! I think you could make an argument that physical cards do in fact contain conceptual cards, and that they're not the same thing, but I would argue that they are.

Necessity of Type-casting

Because PartialCards and WholeCards are both Cards, and there's usually no good reason to separate them out, I'd normally just be working with Collection<Card>. So sometimes I'd need to cast PartialCards in order to access their additional methods. Right now, I'm using the system described here because I really don't like explicit casts. And like Card, Representative would need be cast to either a WholeCard or Composite, to access the actual Cards they represent.

So just for summary:

  • Base type Representative
  • Base type Card
  • Type WholeCard extends Card, Representative (no access needed, it represents itself)
  • Type PartialCard extends Card (gives access to other part)
  • Type Composite extends Representative (gives access to both parts)

Is this insane? I think it actually makes a lot of sense, but I'm honestly not sure.

  • 1
    Are PartialCards effectively cards other than that there are two per physical card? Does the order they are played matter? Could you not just make a "Deck Slot" and "WholeCard" class and allow DeckSlots to have multiple WholeCards, and then just do something like DeckSlot.WholeCards.Play and if there are 1 or 2 play both of them as if they are separate cards? It seems given your much more complicated design there must be something I'm missing :) – enderland Sep 24 '15 at 21:22
  • I really don't feel like I can use polymorphism between whole cards and partial cards to solve this problem. Yes, if I wanted a "play" function, then I could implement that easily, but the problem is partial cards and whole cards have different sets of attributes. I really need to be able to view these objects as what they are, not just as their base class. Unless there's a better solution to that problem. But I've found that polymorphism doesn't mix well with types who share a common base class, but have different methods among them, if that makes sense. – codebreaker Sep 24 '15 at 21:28
  • 1
    Honestly, I think this is ridiculously complicated. You seem to only model "is a" relationships, leading to a very rigid design with tight coupling. More interesting would be what those cards are actually doing? – Daniel Jour Sep 24 '15 at 21:30
  • 2
    If they're "just data objects", then my instinct would be to have one class which contains an array of objects of a second class, and leave it at that; no inheritance or other pointless complications. Whether those two classes should be PlayableCard and DrawableCard or WholeCard and CardPart I have no idea, because I don't know nearly enough about how your game works, but I'm sure you can think of something to call them. – Ixrec Sep 24 '15 at 21:34
  • 1
    What kind of properties do they hold? Can you give examples of actions that use these properties? – Daniel Jour Sep 24 '15 at 21:35
14

It seems to me that you should have a class like

class PhysicalCard {
    List<LogicalCard> getLogicalCards();
}

Code that's concerned with the physical card can deal with the physical card class, and code that's concerned with the logical card can deal with that.

I think you could make an argument that physical cards do in fact contain conceptual cards, and that they're not the same thing, but I would argue that they are.

It doesn't matter whether or not you think the the physical and logic card are the same thing. Don't assume that just because they are the same physical object, they should be the same object in your code. What matters is whether adopting that model makes the coder easier to read and write. The fact is, adopting a simpler model where every physical card is treated as a collection of logical card consistently, 100% of time, will result in simpler code.

  • 2
    Upvoted because this is the best answer, though not for the reason given. This is the best answer not because it is simple, but because physical cards and conceptual cards are not the same thing, and this solution correctly models their relationship. It's true that a good OOP model does not always reflect physical reality, but it should always reflect conceptual reality. Simplicity is good, of course, but it takes a back seat to conceptual correctness. – Kevin Krumwiede Sep 25 '15 at 8:18
  • 2
    @KevinKrumwiede, as I see it there is not a single conceptual reality. Different people think of the same thing in different ways, and people can change how they think of it. You can think of physical and logical cards as separate entities. Or you can think of the split cards as some sort of exception to the general notion of a card. Neither is intrinsically incorrect, but the one lends itself to simpler modelling. – Winston Ewert Sep 25 '15 at 15:42
8

To be blunt, I think the proposed solution is too restrictive and too contorted and disjointed from the physical reality is models, with little advantage.

I'd suggest one of two alternatives:

Option 1. Treat it as single card, identified as Half A // Half B, like the MTG site lists Wear // Tear. But, permit your Card entity to contain N of each attribute: playable-name, mana cost, type, rarity, text, effects, etc..

interface Card {
  List<String> Names();
  List<ManaCost> Costs();
  List<CardTypes> Types();
  /* etc. */
}

Option 2. Not all that dissimilarly from Option 1, model it after the physical reality. You have a Card entity that represents a physical card. And, it's purpose is then to hold N Playable things. Those Playable's can each have a distinct name, mana cost, list of effects, list of abilities, etc.. And your "physical" Card can have its own identifier (or name) that is a compound of each Playable's name, much like the MTG database appears to do.

interface Card {
  String Name();
  List<Playable> Playables();
}

interface Playable {
  String Name();
  ManaCost Cost();
  CardType Type();
  /* etc. */
}

I think either of these options are pretty close to the physical reality. And, I think that'll be beneficial to anyone who looks at your code. (Like your own self in 6 months.)

5

The purpose of these objects is really just to hold properties of the card. They don't really do anything by themselves.

This sentence is a sign that there is something wrong in your design: in OOP, each class should have exactly one role, and the lack of behaviour reveals a potential Data Class, which is a bad smell in the code.

After all, they are cards that represent themselves!

IMHO, it sounds a little bit strange, and even a little bit weird. An object of type "Card" should represent a card. Period.

I know nothing about Magic: The gathering, but I guess you want to use your cards in a similar way, whatever their actual structure is: you want to display a string representation, you want to compute an attack value, etc.

For the problem you describe, I would recommend a Composit Design Pattern, despite the fact this DP is typically presented for solving a more general problem:

  1. Create a Card interface, as you already did.
  2. Create a ConcreteCard, that implements Card and defines a simple face card. Don't hesitate to put the behaviour of a normal card in this class.
  3. Create a CompositeCard, that implements Card and has two additional (and a priori private) Cards. Lets call them leftCard and rightCard.

The elegance of the approach is that a CompositeCard contains two cards, which themselves can be either ConcreteCard or CompositeCard. In your game, leftCard and rightCard will probably systematically be ConcreteCards, but the Design Pattern allows you to design higher level compositions for free if you want. Your card manipulation will not take into account the real type of your cards, and therefore you don't need things such as casting to subclass.

CompositeCard must implement the methods specified in Card, of course, and will do it by taking into account the fact that such a card is made of 2 cards (plus, if you want, something specific to the CompositeCard card itself. For instance, you may want the following implementation:

public class CompositeCard implements Card
{ 
   private final Card leftCard, rightCard;
   private final double factor;

   @Override // Defined in Card
   public double attack(Player p){
      return factor * (leftCard.attack(p) + rightCard.attack(p));
   }

   @Override // idem
   public String name()
   {
       return leftCard.name() + " combined with " + rightCard.name();
   }

   ...
}

By doing that, you can use a CompositeCard exactly as you do for any Card, and the specific behaviour is hidden thanks to polymorphism.

If you are sure a CompositeCard will always contain two normal Cards, you can keep the idea, and simply use ConcreateCard as a type for leftCard and rightCard.

  • You are talking about the composite pattern, but in fact since you're holding two references of Card in CompositeCard you are implementing the decorator pattern. I also recommand to the OP to use this solution, decorator is the way to go ! – Spotted Sep 25 '15 at 6:05
  • I don't see why having two Card instances makes my class a decorator. According to your own link, a decorator adds features to a single object, and is itself an instance of the same class/interface than this object. While, according to your other link, a composite allows the owning of multiple objects of the same class/interface. But ultimately the words don't matter, and only the idea is great. – mgoeminne Sep 25 '15 at 7:33
  • @Spotted This is definitely not the decorator pattern as the term is used in Java. The decorator pattern is implemented by overriding methods on an individual object, creating an anonymous class that is unique to that object. – Kevin Krumwiede Sep 25 '15 at 8:23
  • @KevinKrumwiede as long as CompositeCard doesn't expose additional methods, CompositeCard is only a decorator. – Spotted Sep 25 '15 at 8:42
  • "... Design Pattern allows you to design higher level compositions for free" - no, it does not come for free, just the opposite - it comes for the price of having a solution which is more complicated than needed for the requirements. – Doc Brown Sep 25 '15 at 12:07
3

Maybe everything is a Card when it's in the deck or the graveyard, and when you play it, you construct a Creature, Land, Enchantment, etc. from one or more Card objects, all of which implement or extend Playable. Then, a composite becomes a single Playable whose constructor takes two partial Cards, and a card with a kicker becomes a Playable whose constructor takes a mana argument. The type reflects what you can do with it (draw, block, dispel, tap) and what can effect it. Or a Playable is just a Card that has to be carefully reverted (losing any bonuses and counters, getting split up) when taken out of play, if it’s really useful to use the same interface to invoke a card and predict what it does.

Perhaps Card and Playable have-a effect.

  • Unfortunately, I don't play these cards. They're really just data objects that can be queried. If you want a deck data structure, you want a List<WholeCard> or something. If you want to perform a search for green Instant cards, you'd want a List<Card>. – codebreaker Sep 24 '15 at 21:48
  • Ah, okay. Not very relevant to your problem, then. Should I delete? – Davislor Sep 24 '15 at 21:52
3

The visitor pattern is a classic technique for recovering hidden type information. We can use it (a slight variation of it here) here to discern between the two types even when they are stored in higher-abstraction variables.

Let's start with that higher abstraction, a Card interface:

public interface Card {
    public void accept(CardVisitor visitor);
}

There may be a little more behavior on the Card interface, but most of the property getters move to a new class, CardProperties:

public class CardProperties {
    // property methods, constructors, etc.

    String name();
    String text();
    // ...
}

Now we can have a SimpleCard representing a whole card with a single set of properties:

public class SimpleCard implements Card {
    private CardProperties properties;

    // Constructors, ...

    @Override
    public void accept(CardVisitor visitor) {
        visitor.visit(properties);
    }
}

We see how the CardProperties and the yet-to-be-written CardVisitor are starting to fit in. Let's do a CompoundCard to represent a card with two faces:

public class CompoundCard implements Card {
    private CardProperties firstFaceProperties;
    private CardProperties secondFaceProperties;

    // Constructors, ...

    public void accept(CardVisitor visitor) {
        visitor.visit(firstFaceProperties, secondFaceProperties);
    }
}

The CardVisitor starts to emerge. Let's try to write that interface now:

public interface CardVisitor {
    public void visit(CardProperties properties);
    public void visit(CardProperties firstFaceProperties, CardProperties secondFaceProperties);
}

(This is a first version of the interface for now. We can make improvements, which will be discussed later.)

We've now fleshed out all the parts. We now just need to put them together:

List<Card> cards = new LinkedList<>();
cards.add(new SimpleCard(new CardProperties(/* ... */)));
cards.add(new CompoundCard(new CardProperties(/* ... */), new CardProperties(/* ... */)));

 for(Card card : cards) {
     card.accept(new CardVisitor() {
         @Override
         public void visit(CardProperties properties) {
             // Do something for simple cards with a single face
         }

         public void visit(CardProperties firstFaceProperties, CardProperties secondFaceProperties) {
             // Do something else for compound cards with two faces
         }
     });
 }

The runtime will handle the dispatch to the correct version of the #visit method through polymorphism rather than trying to break it.

Rather than use an anonymous class, you can even promote the CardVisitor to an inner class or even a full class if the behavior is reusable or if you want the ability to swap behavior at runtime.


We can use the classes as they are now, but there are some improvements we can make to the CardVisitor interface. For example, there may come a time when Cards can have three or four or five faces. Rather than adding new methods to implement, we could just have the second method take and array instead of two parameters. This makes sense if multi-faced cards are treated differently, but the number of faces above one are all treated similarly.

We could also convert CardVisitor to an abstract class instead of an interface, and have empty implementations for all the methods. This allows us to implement only the behaviors we are interested in (maybe we are only interested in single-faced Cards). We can also add new methods without forcing every existing class to implement those methods or fail to compile.

  • 1
    I think this could be easily expanded to the other kind of two faced card (front & back instead of side by side). ++ – RubberDuck Sep 25 '15 at 0:52
  • For further exploration, the use of a Visitor for exploiting methods specific to a subclass is sometimes called Multiple Dispatch. Double Dispatch could be an interesting solution to the problem. – mgoeminne Sep 25 '15 at 5:42
  • 1
    I'm voting down because I think the visitor pattern adds unnecessary complexity and coupling to the code. Since an alternative is available (see mgoeminne's answer), I wouldn't use it. – Spotted Sep 25 '15 at 9:05
  • @Spotted The visitor is a complex pattern, and I also considered the composite pattern when writing the answer. The reason I went with the visitor is because the OP wants to treat similar things differently, rather than different things similarly. If the cards had more behavior, and were being used for gameplay, then the composite pattern allows for combining data to produce a unified stat. They are just bags of data, though, possibly used render a card display, in which case getting separated information seemed more useful than a simple composite aggregator. – cbojar Sep 25 '15 at 12:55

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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