After I posted my last blog post, my code was bothering me. It had code smell.

I wanted to get something started knowing I would go back and refactor things, but it still feels kind of bad to post what I now consider to be bad code, but that is kind of the point of this series of posts.

Good code, to quote a friend of mine, doesn’t spring from the mind fully formed like Athena from the head of Zeus. It is an iterative approach where you get something out there and you work with it and figure out what you didn’t consider when you were theoretically approaching the code as an abstract concept.

Separating the Responsibilities

Part of the issue I have with architecting this piece of software was figuring out how to represent things in code. I want to avoid spaghetti code. I also want to avoid making something overly complicated.

I feel that my initial foray stripped out some necessary complexity.

I was trying to keep my card class as simple as possible. Without looking at the rules too carefully, I initial felt that it was only necessary to track the suit and the type of card:

struct Card {
    let cardSuit:CardSuit
    let cardType:CardType
    
    init(suit:CardSuit, type:CardType) {
        cardSuit = suit
        cardType = type
    }
}

While I was going through the initialization process for these cards, I realized that there were a few edge cases I didn’t take into consideration:

• Collecting all of the bird cards gives a bonus
• The ribbons are different colors and collecting all three ribbons of the same color is worth more than three different colored ribbons
• Some of the Junk cards are worth two cards instead of one

I tried to address that in the key I assigned each card. Every card is unique and needed a unique identifier, so I figured I would represent these edge cases in the key, but that was a really big mistake.

”Stringly”-Typed and Safety

I was making everything in my deck, as Justin Williams says, “stringly”-typed.

Stringly-typed occurs when you are using a key to correlate to something rather than a property that can be auto-completed. Core Image is (or at least used to be) stringly-typed.

I was basing all of my programming logic in this application upon the hope that I would remember my own naming scheme and not misspell anything. That is a bad assumption.

Additionally, I was separating out the responsibility in the wrong place. The card should know everything about itself. It should know its type. It should know its suit. If it is an edge case, it should know that too. It should not be the responsibility of the programming logic to know that a card has a blue ribbon or not. The card should know that about itself.

So how do I deal with that?

Optionals?

At first I thought optionals would be a good way to approach this. I want to know if an animal card is a bird or not. I also want to know what color the ribbon is and if the junk card is worth two or not. I can make these optional properties on my Card Struct:

struct Card {
    let cardSuit:CardSuit
    let cardType:CardTyped
    let ribbonColor:RibbonColor?
    let isBird:Bool?
    let isDouble:Bool?
    
    init(suit:CardSuit, type:CardType) {
        cardSuit = suit
        cardType = type
    }

    init(suit:CardSuit, type:CardType, ribbon:RibbonColor) {
        cardSuit = suit
        cardType = type
	 ribbonColor = ribbon
    }

    init(suit:CardSuit, type:CardType, bird:Bool) {
        cardSuit = suit
        cardType = type
	 isBird = bird
    }

    init(suit:CardSuit, type:CardType, double:Bool) {
        cardSuit = suit
        cardType = type
	 isDouble = double
    }
}

This creates a few different initializers to accommodate each of the edge cases. Great, right?

Not really.

As I figure out more and more edge cases, the larger and more complex this struct is going to get. Also, none of these edge cases overlap. There will never be a ribbon card that is worth double or will have a bird on it.

So what is a better approach?

Protocols

It makes more sense to, instead of having an enum of different card types, to create a Card protocol that includes only what every card will require and then create different structs for each type of card that can be tailored to its specific edge cases and requirements.

The simplest card type, the one that has no edge cases associated with it, is the Bright type. All a Bright card needs to know is that it is a Bright and what suit it is. Since I can make a Bright struct that knows it is a Bright card, the only thing the protocol needs (that I am aware of) is a suit. I can expand on that later if I figure out further into the project that this was too simple.

protocol Card {
    var cardSuit:CardSuit {get}
    var cardName:String {get}
}

I am adding a cardName property because I want to eventually incorporate these structures into a command line program and later add a user interface. I would like to have something to display in the command line and later have a key that I can use to assign the correct card in the UI. I had that in my previous implementation, but I want to move it into the class because a few cards will be duplicates and I want to not have to make unique identifiers for everything, especially when the cards are the same.

Now that I have a Card protocol, I can apply it to four different structs that are tailored for each card type and its own edge cases.

struct BrightCard:Card {
    
    internal var cardSuit: CardSuit
    internal var cardName: String

    init(suit: CardSuit, name: String) {
        cardSuit = suit
        cardName = name
    }
}

struct RibbonCard:Card {
    
    internal var cardSuit: CardSuit
    internal var cardName: String
    internal var ribbonColor: RibbonColor
    
    init(suit: CardSuit, name: String, ribbon: RibbonColor) {
        cardSuit = suit
        cardName = name
        ribbonColor = ribbon
    }
}

struct AnimalCard:Card {
    
    internal var cardSuit: CardSuit
    internal var cardName: String
    internal var isBird: Bool
    
    init(suit: CardSuit, name: String, bird:Bool) {
        cardSuit = suit
        cardName = name
        isBird = bird
    }
    
}

struct JunkCard:Card {
    
    internal var cardSuit: CardSuit
    internal var cardName: String
    internal var isDouble: Bool
    
    init(suit: CardSuit, name: String, double:Bool) {
        cardSuit = suit
        cardName = name
        isDouble = double
    }
}

If I am working on the game logic and find another edge case, I can easily apply it to its specific card type rather than making it a global optional or having to reflect it in the card’s name.

Since every one of these cards conforms to the same protocol, I can create an array of anything conforming to the Card protocol.

let deck:[Card] = [BrightCard(suit: .January, name:"JanuaryBright"),
                   BrightCard(suit: .March, name:"MarchBright"),
                   BrightCard(suit: .August, name: "AugustBright"),
                   BrightCard(suit: .November, name: "NovemberBright"),
                   BrightCard(suit: .December, name: "DecemberBright"),
                   RibbonCard(suit: .January, name: "JanuaryPurpleRibbon", ribbon: .Purple),
                   RibbonCard(suit: .February, name: "FebruaryPurpleRibbon", ribbon: .Purple),
                   RibbonCard(suit: .March, name: "MarchPurpleRibbon", ribbon: .Purple),
                   RibbonCard(suit: .April, name: "AprilRedRibbon", ribbon: .Red),
                   RibbonCard(suit: .May, name: "MayRedRibbon", ribbon: .Red),
                   RibbonCard(suit: .July, name: "JulyRedRibbon", ribbon: .Red),
                   RibbonCard(suit: .December, name: "DecemberRedRibbon", ribbon: .Red),
                   RibbonCard(suit: .June, name: "JuneBlueRibbon", ribbon: .Blue),
                   RibbonCard(suit: .September, name: "SeptemberBlueRibbon", ribbon: .Blue),
                   RibbonCard(suit: .October, name: "OctoberBlueRibbon", ribbon: .Blue),
                   AnimalCard(suit: .February, name: "FebruaryBird", bird: true),
                   AnimalCard(suit: .April, name: "AprilBird", bird: true),
                   AnimalCard(suit: .May, name: "MayAnimal", bird: false),
                   AnimalCard(suit: .June, name: "JuneAnimal", bird: false),
                   AnimalCard(suit: .July, name: "JulyAnimal", bird: false),
                   AnimalCard(suit: .August, name: "AugustBird", bird: true),
                   AnimalCard(suit: .September, name: "SeptemberAnimal", bird: false),
                   AnimalCard(suit: .October, name: "OctoberAnimal", bird: false),
                   AnimalCard(suit: .December, name: "DecemberAnimal", bird: false),
                   JunkCard(suit: .January, name: "JanuaryJunk", double: false),
                   JunkCard(suit: .January, name: "JanuaryJunk", double: false),
                   JunkCard(suit: .February, name: "FebruaryJunk", double: false),
                   JunkCard(suit: .February, name: "FebruaryJunk", double: false),
                   JunkCard(suit: .March, name: "MarchJunk", double: false),
                   JunkCard(suit: .March, name: "MarchJunk", double: false),
                   JunkCard(suit: .April, name: "AprilJunk", double: false),
                   JunkCard(suit: .April, name: "AprilJunk", double: false),
                   JunkCard(suit: .May, name: "MayJunk", double: false),
                   JunkCard(suit: .May, name: "MayJunk", double: false),
                   JunkCard(suit: .June, name: "JuneJunk", double: false),
                   JunkCard(suit: .June, name: "JuneJunk", double: false),
                   JunkCard(suit: .July, name: "JulyJunk", double: false),
                   JunkCard(suit: .July, name: "JulyJunk", double: false),
                   JunkCard(suit: .August, name: "AugustJunk", double: false),
                   JunkCard(suit: .August, name: "AugustJunk", double: false),
                   JunkCard(suit: .September, name: "SeptemberJunk", double: false),
                   JunkCard(suit: .September, name: "SeptemberJunk", double: false),
                   JunkCard(suit: .October, name: "OctoberJunk", double: false),
                   JunkCard(suit: .October, name: "OctoberJunk", double: false),
                   JunkCard(suit: .November, name: "NovemberJunk", double: false),
                   JunkCard(suit: .November, name: "NovemberJunk", double: false),
                   JunkCard(suit: .November, name: "NovemberJunkDouble", double: true),
                   JunkCard(suit: .December, name: "DecemberJunkDouble", double: true)]

I know that dictionaries are faster than arrays to deal with, but this isn’t a situation where the array is going to constantly increase in size. These cards have been stable for a few hundred years, they are not going to dramatically change in any way.

Approaching this in this way has made my code a lot easier to deal with. I can sort all of the cards in the array by their type so that when I get around to writing the scoring logic it will be easier to simply pull out every card by its type. Anything I want to track, such as if a junk card is worth two, can be added to this set of code that I never have to touch again. I don’t have to remember how I named anything because I can track everything through its type and its properties. I kept the names so that when I read what cards I have in my hand I know what they are without having to remember what they’re called while I am doing my programming logic.

Conclusions

My new playground with the refactored code is here.

I wasn’t planning to refactor this code this early in the process, but I feel like this is a good illustration of why I am writing this series of posts.

I have seen many projects where they were built upon faulty assumptions. Rather than going back and rewriting the code, work-arounds were implemented making the code increasingly more complex and fragile.

Had I continued on the path I was on, the code base would have been incredibly difficult to maintain. There are probably still better ways to approach this, but I feel a lot better about the code currently than I did earlier this week. I want to idiot proof this code as much as possible and I stripped out some necessary complexity without thinking about how much harder that would make things for me later.

My next goal with this code is to figure out the best way to shuffle the array containing the deck. After I get that squared away, I need to figure out how to represent a player, a hand, and the game board. I also need to start writing unit tests for my code, but I will wait until later. No, I am not doing TDD. Jump in a river.

Until next time!