I'm thinking about a structure like this.

        - john:
                - card:
                    id: 1
                    suit: diamond
                    number: 7
        - id: 32
          suit: spade
          number: 3

And then, to draw a card there would be a drawCard(cardGame, john) that pops the card from the deck and appends it to to john.hand. And then say, if John were to place the card at the bottom of the deck, there would be a sequence of object manipulations, array deletions, etc.

But looking at this, I see risk of accidentally duplicating a card object or running into deep vs. shallow copy issues... is it better to maintain a "flat" structure that is almost like a normalized database table? Ex:

        - id: 1
          suit: diamond
          number: 7
          heldPlayer: john
        - id: 32
          suit: spade
          number: 3
          heldPlayer: null

And in this case, it would be easy for players to swap cards or remove from hands without making any structural changes, though it may be harder to deal with deck positionality? (This could be a failure of imagination on my part)

Intuitively I'd go with the first structure... but I'm not sure what would be better from a stability perspective.

  • 3
    Moving objects between lists is ok, for many tasks such as this it is a perfect fit. Duplicates don't happen by accident, but by bugs in your code, so keep the parts of your code that move cards around short, clear and well-tested, and you should be fine. Jul 25 '19 at 5:43
  • a lot depends on the data structure you chose for storing the cards. e.g. if the entire deck could be placed in a doubly ended queue it could safeguard against any duplication assuming there's no duplication when the queue was formed and the cards are only placed/removed from either of the ends Jul 25 '19 at 6:01
  • 1
    In OOP you would encapsulate that behaviour - e.g. a Deck class with a private list, not exposing a method removeCard(Card card) that can go wrong in all sorts of ways, but instead a method moveCard(Card card, Deck target). @FPers, what does FP do in this case instead? Because that's probably what you should do.
    – R. Schmitz
    Jul 25 '19 at 10:16
  • I figured if you stick to the paradigm of "return a new object and dont modify the original" you can at least have more consistent behavior on the object... looking at this (haskell) example cse.chalmers.se/edu/year/2017/course/TDA555/lab2.html Jul 25 '19 at 15:29

I would say that, yes, this is a code smell, but for the more specific reason of "using lists of things to represent locations of things"

There are plenty of reasons to have lists of things and to move objects in and out of them, queues, caches, sets of things etc. But as you point out its inherently possible to have the same object in multiple collections or none.

In your case and the case of "items in physical locations" in general you need both a single "location" AND multiple references to the same object in the code.

So for example, when displaying the card in your game UI it might be in more than one place, eg in the hand of the player avatar as a 3d model, in the inventory window as a 2d sprite, in the history of plays sidebar etc etc.

If Deck.Cards is conceptually a physical location, but PlayHistory.Cards isn't. You have no way to really enforce that in the code and may well find that some parts of your code base want to have the card in a single list and some want it referenced in multiple lists

If you keep a single list of AllCards and give them Ids, you can then have a CardLocation dictionary which enforces a single location per card for a particular part of your logic without affecting the rest of the code

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.