patternjavascriptModerate
"War" card game which includes betting "money" as well
Viewed 0 times
includesbettingmoneywarcardgamewellwhich
Problem
I have created a JavaScript based game, which is based on the card game War. I think I am reusing a lot of my code, but I am not entirely sure what I can really improve. I have a lot of functions, which could probably be improved.
Please take a look and give me a review of it. I think I nailed it, but let me know what you think. Also, it is supposed to run on a Node.js server, so basically everything will be authorized through the server, which will check everything through before giving the user input (which prevents faking their total money, their bettings, etc.).
Note: Some variables may not be used in the code. That's on purpose (most of them are used) because it is for the future of the project.
Also on JSFiddle - To view the actual page Go here!
`//Each player's deck
var dealerDeck = [];
var playerDeck = [];
//52 cards = 26 cards left
var dealerAmountLeft = 26;
var playerAmountLeft = 26;
//The amount of money and the bet amount
var playerTotal = 40;
var betAmount = 0;
//Each round, dealerCard and playerCard are set to a random card from their deck
//warDealerCards/warPlayercards is a string, which contains 1, 2, then 3 cards seperated by a +
var dealerCard, playerCard, warDealerCards, warPlayerCards;
//Checks how many cards has been giving when a war has been started
var warCards = 0;
//Whether a war has been started or not
var warStarted = false;
//Keeps track of how many times each player has won in a started war
var dealerWarsWon = 0;
var playerWarsWon = 0;
var warInterval;
//Sets a language
var language = "en-GB";
//Card class
function Card(value, name, suit) {
this.value = value;
this.name = name;
this.suit = suit;
}
//Deck class
function Deck() {
switch (language) {
case "da-DK":
this.suits = ['Hjerter', 'Ruder', 'Spar', 'Klør'];
this.names = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', 'knægt', 'dronning', 'konge'];
break;
case "en-US":
this.suits =
Please take a look and give me a review of it. I think I nailed it, but let me know what you think. Also, it is supposed to run on a Node.js server, so basically everything will be authorized through the server, which will check everything through before giving the user input (which prevents faking their total money, their bettings, etc.).
Note: Some variables may not be used in the code. That's on purpose (most of them are used) because it is for the future of the project.
Also on JSFiddle - To view the actual page Go here!
`//Each player's deck
var dealerDeck = [];
var playerDeck = [];
//52 cards = 26 cards left
var dealerAmountLeft = 26;
var playerAmountLeft = 26;
//The amount of money and the bet amount
var playerTotal = 40;
var betAmount = 0;
//Each round, dealerCard and playerCard are set to a random card from their deck
//warDealerCards/warPlayercards is a string, which contains 1, 2, then 3 cards seperated by a +
var dealerCard, playerCard, warDealerCards, warPlayerCards;
//Checks how many cards has been giving when a war has been started
var warCards = 0;
//Whether a war has been started or not
var warStarted = false;
//Keeps track of how many times each player has won in a started war
var dealerWarsWon = 0;
var playerWarsWon = 0;
var warInterval;
//Sets a language
var language = "en-GB";
//Card class
function Card(value, name, suit) {
this.value = value;
this.name = name;
this.suit = suit;
}
//Deck class
function Deck() {
switch (language) {
case "da-DK":
this.suits = ['Hjerter', 'Ruder', 'Spar', 'Klør'];
this.names = ['1', '2', '3', '4', '5', '6', '7', '8', '9', '10', 'knægt', 'dronning', 'konge'];
break;
case "en-US":
this.suits =
Solution
Pretty nice - kudos.
If I were to point out some things - which of course I am, this being a review - it'd be your prototype/class structure.
You have a
If you call
So, in essence, you can just as well call
It'd make a lot more sense to only have the
Now, to keep track of multiple cards - i.e. the deck - I'd expand the
With such changes, you'll model the domain a lot more closely. And skip a bunch of code, too.
Incidentally, the "ace is 1 but is really 14"-hack has a fairly simple solution: Change your
Eller, på dansk:
Now an ace will automatically have the highest value, because that's the order you've defined.
On another note, don't do things like this:
You have arrays which know their own length; ask them how many cards are left. Besides, even if you had to manually keep track of it (which you don't), there's no reason to keep track of two separate values, since - in this variant of the game - both players will always have the same number of cards.
Oh, and "amount" is bad name for "number of cards" - especially when you're also dealing with bets and "amount" is typically used to describe money.
In terms of rules, war is usually played without bets, and the winner of each round just "wins" the cards on the table. Hence the reason to put down extra cards in case of a war: More to lose/win. But your implementation of 3 extra cards all being used to determine the outcome is a little strange.
Especially as you're not checking if all those cards happen to be equal too. Haven't checked, but I think everything just grinds to a halt if a war ends in a stalemate. Edit: No, it doesn't; it's just war-time, all the time. But it does grind to a halt if there aren't enough cards to wage war. It tries to draw 3, but runs out.
The more common way to implement things (doubly so when cards are not also treated as currency), is simply to keep drawing cards as long as there's a stalemate.
If I were to point out some things - which of course I am, this being a review - it'd be your prototype/class structure.
You have a
Card constructor, which is cool. But you also have PlayerCard and DealerCard constructors/prototypes. But they're problematic:- They don't actually derive from
Card, though that'd make sense.
- They repeat each other quite a lot.
- They're really just functions in disguise.
If you call
new PlayerCard() you just get an empty object, really. It's not really a "card", just a container for a toString method, which reaches into a global array to find something to return.So, in essence, you can just as well call
PlayerCard.prototype.toString() directly, when you want a card.It'd make a lot more sense to only have the
Card class, since a card is the same regardless of who's holding it. So I'd do:function Card(value, name, suit) {
this.value = value;
this.name = name;
this.suit = suit;
}
Card.prototype = {
toString: function () {
return this.suit + " " + this.name;
}
};Now, to keep track of multiple cards - i.e. the deck - I'd expand the
Deck constructor a little. Namely, I'd add shuffle as prototype method, so you can call deck.shuffle(), and I'd add a deal() method too, which returns and removes a card from the deck. Or you can have deal pick a card at random to remove and return, negating the need for a separate shuffle call. (In fact, you're already doing something like that in selectCards even though you're also shuffling the deck.)With such changes, you'll model the domain a lot more closely. And skip a bunch of code, too.
Incidentally, the "ace is 1 but is really 14"-hack has a fairly simple solution: Change your
names array (typically called "ranks" when talking about playing cards) to the order you're actually using:['2', '3', '4', '5', '6', '7', '8', '9', '10', 'Jack', 'Queen', 'King', 'Ace']Eller, på dansk:
['2', '3', '4', '5', '6', '7', '8', '9', '10', 'knægt', 'dame', 'konge', 'es']Now an ace will automatically have the highest value, because that's the order you've defined.
On another note, don't do things like this:
dealerAmountLeft = 26;
//...
dealerAmountLeft--;You have arrays which know their own length; ask them how many cards are left. Besides, even if you had to manually keep track of it (which you don't), there's no reason to keep track of two separate values, since - in this variant of the game - both players will always have the same number of cards.
Oh, and "amount" is bad name for "number of cards" - especially when you're also dealing with bets and "amount" is typically used to describe money.
In terms of rules, war is usually played without bets, and the winner of each round just "wins" the cards on the table. Hence the reason to put down extra cards in case of a war: More to lose/win. But your implementation of 3 extra cards all being used to determine the outcome is a little strange.
Especially as you're not checking if all those cards happen to be equal too. Haven't checked, but I think everything just grinds to a halt if a war ends in a stalemate. Edit: No, it doesn't; it's just war-time, all the time. But it does grind to a halt if there aren't enough cards to wage war. It tries to draw 3, but runs out.
The more common way to implement things (doubly so when cards are not also treated as currency), is simply to keep drawing cards as long as there's a stalemate.
Code Snippets
function Card(value, name, suit) {
this.value = value;
this.name = name;
this.suit = suit;
}
Card.prototype = {
toString: function () {
return this.suit + " " + this.name;
}
};['2', '3', '4', '5', '6', '7', '8', '9', '10', 'Jack', 'Queen', 'King', 'Ace']['2', '3', '4', '5', '6', '7', '8', '9', '10', 'knægt', 'dame', 'konge', 'es']dealerAmountLeft = 26;
//...
dealerAmountLeft--;Context
StackExchange Code Review Q#114563, answer score: 10
Revisions (0)
No revisions yet.