Design a simple card game in JavaScript

Question

I wrote a simple card game in JavaScript and I wonder if anyone could give me some advice and make some improvements on my code.

Here is the demo that I build: https://codesandbox.io/s/cardgame-his95?file=/src/index.js

So it is a 2 person card game. Every card has a number and also has a pattern. The game goes like this: we have a set of pre-defined rules, and these rules have a ranking. The player that satisfies the highest-ranked rule will win. And there could be tie if they end up with the same ranking. The goal here is not just to make the game work, also I wanted to account for maintainability. For example, we can easily add new rules and swap the ranking and re-define the rule if possible.

There are mainly a couple of classes. First is the Game class

class Game {
 constructor({ play1, play2, rules, messages }) {
 this.play1 = play1;
 this.play2 = play2;
 this.rules = rules;
 this.messages = messages;
 }
 play() {
 let rankOfP1 = Infinity;
 let rankOfP2 = Infinity;
 for (const rule of this.rules) {
 if (rule.validator(this.play1)) {
 rankOfP1 = rule.rank;
 break;
 }
 }
 for (const rule of this.rules) {
 if (rule.validator(this.play2)) {
 rankOfP2 = rule.rank;
 break;
 }
 }
 return rankOfP1 === rankOfP2
 ? this.messages.tie
 : rankOfP1 < rankOfP2
 ? this.messages.win.player1
 : this.messages.win.player2;
 }
}

Here the rules are an array of rule object where every object looks like this

{
 description: "Six Cards of the same pattern",
 rank: 1,
 validator: cards => {
 return hasSamePattern(cards, 6);
 }

The lower the rank gets, the more important this rule is. So If player1 satisfies a rule with rank 1, and player1 satisfies a rule with rank 2, then we say player1 won. And validator is the function that takes an array of card object and returns a boolean to determine if the set of cards satisfy the rule.

And lastly we have a Card class which is pretty simple

class Card {
 constructor({ pattern, number }) {
 this.pattern = pattern;
 this.number = number;
 }
}

Please take a look and feel free to make some improvements on it. Also please suggest some better naming for variables if possible, I am not a native English speaker so some of the names of the variables would be a bit weird.

Finally I wrote this game in a OOP style. I know that JavaScript is not the best language out there when it comes to OOP. Also I am not really good at Object Oriented Design. I wonder if anyone knows how to rewrite the game in functional programming style. That would be super cool!

Answer 1

Your constructor has

constructor({ play1, play2, rules, messages }) {
 this.play1 = play1;
 this.play2 = play2;
 this.rules = rules;
 this.messages = messages;
}

You may as well Object.assign the parameter to the instance instead:

constructor(config) {
 Object.assign(this, config);
}

pattern is a slightly odd name for what it represents here - the usual English word for one of the clubs, diamonds, etc, is suit. rule is a bit strange as well - a rule usually refers to the process by which a game is played (eg "Hands consist of 6 cards" or "The player with the best hand wins"). To describe the different possible winning combinations and their ranks, I'd use the word handRanks or something similar. play1 and play2 aren't great descriptors either - these represent the cards held in each player's hand, so maybe use player1Cards or player1Hand.

With regards to the play() method, when you want to find an item in an array which fulfills a condition, it would be more appropriate to use .find, rather than a for loop - find more clearly indicates what the intention of the loop is, and is more concise. You also need to set the rank to Infinity if no handRanks pass - why not integrate this Infinity into the handRanks array itself? You're also writing the looping code twice - you can make it more DRY by putting it into a function, and calling that function twice instead.

new Card({ suit: "spade", number: 1 }), // <-- Suit

new HandRank({ // <-- HandRank
 description: "Six Cards of the same suit", // <-- Suit
 rank: 1,
 validator: cards => {
 return hasSameSuit(cards, 6); // <-- hasSameSuit, not hasSamePattern
 }
}),
new HandRank({ // <-- HandRank
 description: "Nothing special",
 rank: Infinity, // <-- add this whole new HandRank
 validator: cards => true,
}),

getRank(cards) {
 return this.handRanks.find(({ validator }) => validator(cards)).rank; // <-- this.handRanks
}
play() {
 const rankOfP1 = this.getRank(this.player1Cards); // <-- player1Cards
 const rankOfP2 = this.getRank(this.player2Cards); // <-- player2Cards
 return rankOfP1 === rankOfP2
 ? this.messages.tie
 : rankOfP1 < rankOfP2
 ? this.messages.win.player1
 : this.messages.win.player2;
}

One of the benefits of using arrow functions is that if the function contains only a single expression which is immediately returned, you can omit the { } brackets and the return keyword, if you want to make things concise, eg for the hasSameSuit test above:

validator: cards => hasSameSuit(cards, 6),

If you want to find if any item in an array passes a test, but you don't care about which item passes the test, you should use .some, not .find. (.some returns a boolean indicating whether any passed, .find returns the found item) For the hasSamePattern (or hasSameSuit) method, use:

return Object.values(patterns).some(num => num >= threshold);

Your hasConsecutiveNums method has the bug mentioned in the comments previously - a hand of [1, 2, 2, 3] will not pass a 3-consecutive-number test because the sorted array will contain 2 twice, failing if (prevNum + 1 === num) {. De-duplicate the numbers with a Set first.

const nums = [...new Set(cards.map(card => card.number).sort((a, b) => a - b))];

I wonder if anyone knows how to rewrite the game in functional programming style.

Javascript isn't entirely suited to completely functional programming either, though it can get most of the way there. To start with, make your functions pure, and avoid side-effects and mutations. For example, assigning to a property of the instance with this.play1 = play1; (or this.player1Cards = player1Cards;) is a mutation. None of your code fundamentally requires anything non-functional (except the console.log at the very end, which is unavoidable), so it should be pretty easy to convert - rather than assigning to properties, just keep the variables in a closure, and return a function for the play method, eg:

const makeGame = ({ player1Cards, player2Cards, handRanks, messages }) => () => {
 // getRank is now a standalone function which takes a handRanks parameter
 const rankOfP1 = getRank(player1Cards, handRanks);
 const rankOfP2 = getRank(player2Cards, handRanks);
 return rankOfP1 === rankOfP2
 ? messages.tie
 : rankOfP1 < rankOfP2
 ? messages.win.player1
 : messages.win.player2;
};

const play = makeGame({ ... });
console.log(play());

• Joji: Hi. Thanks for such a thorough answer! I'd like to ask more questions about functional programming. In your opinion, is it better to write this game in a functional programming style or OO style? If we were using functional programming here. Do we still need to use new to initialize all those objects like card objects, player objects, and HandRank object?
• CertainPerformance: Both work, it's purely a matter of preference. Personally, I'd strongly prefer to go the functional method, but someone else might think differently. Do we still need to use new No, usually, if you're trying to be functional, you should not use new, because that indicates that you're mutating somewhere.
• CertainPerformance: But in this script, your new Card and new Rules don't accomplish anything useful - all they do is create objects with properties. Using a plain object instead would make more sense IMO. Eg const rules = [ { description: "Six Cards of the same pattern", rank: 1, validator: cards => { return hasSamePattern(cards, 6); } }, ... Classes are useful when you want to bundle data with methods. Data alone or functions alone don't have a reason to be in classes IMO. Use a class when you need data and methods associated with the data.
• Joji: if we ditch new, but I find using object literal doesn't scale well and you need to write it every time you need a new card or rule, is there a functional way to produce objects? And can you show me how to do it in the code?
• CertainPerformance: When all you need is a data structure, like here, using an object literal scales better than using new - it requires less code and (IMO) is easier to understand intuitively (compared with having to go to the constructor definition to see what's going on).new Card({ suit: "spade", number: 1 }) vs { suit: "spade", number: 1 } - since Card doesn't have any methods, there's no point to the new.