patternjavascriptMinor
How'd I do on this card matching game?
Viewed 0 times
thiscardgamehowmatching
Problem
I threw this together for a web dev class I am taking at school and I thought I'd throw it up to see what criticism I could get on my JavaScript, design, whatever.
Bonus points if you can tell me how I can fix the bug with users clicking to quickly and more than two cards flipping at a time.
Disclaimer: I know that I left the cheat in so you can get the first set of cards. I didn't want anybody going crazy before getting that first set.
Here is the Card Matching Game.
```
// Author: Matt Gowie
// Created on: 10/01/12
// Project for Web Dev 2400
$(document).ready(function(){
var cardArray =
[
'd-ace', 'd-ace', 's-ace', 's-ace', 'c-ace',
'c-ace', 'h-ace', 'h-ace', 'd-king', 'd-king',
's-king', 's-king', 'c-king', 'c-king', 'h-king',
'h-king', 'd-quen', 'd-quen', 's-quen', 's-quen',
'c-quen', 'c-quen', 'h-quen', 'h-quen', 'd-jack',
'd-jack', 's-jack', 's-jack', 'c-jack', 'c-jack',
'h-jack', 'h-jack', 'd-ten', 'd-ten', 's-ten',
's-ten', 'c-ten', 'c-ten', 'h-ten', 'h-ten',
'd-nine', 'd-nine', 's-nine', 's-nine', 'c-nine',
'c-nine', 'h-nine', 'h-nine', 'd-sev', 'd-sev',
's-sev', 's-sev', 'c-sev', 'c-sev', 'h-sev',
'h-sev', 'd-six', 'd-six', 'joker', 'joker'
];
var cardsToFlip = [];
var matches = 0;
var score = 0;
var multiplyer = 10;
fillBoard = function(){
$('.board').html('')
var rowCount = 1;
var colCount = 1;
var card = '';
for(var i = 1; i ').attr('id', "card" + i)
.addClass("row" + rowCount).addClass("col" + colCount);
var $cardContainer = $('')
.data('cardSide', 'back').append($card);
$container.html($cardContainer);
$('.board').append($container);
colCount += 1;
if(i % 10 === 0){ rowCount += 1; colCount = 1; }
}
}();
$('.container').bind('click', function(){
var cardId = $(this).attr('id');
if(cardsToFlip.length <= 1 && cardsToFlip[0] !== cardId){
flipCard(this);
cardsToFlip.push(cardId);
} else {
if(
Bonus points if you can tell me how I can fix the bug with users clicking to quickly and more than two cards flipping at a time.
Disclaimer: I know that I left the cheat in so you can get the first set of cards. I didn't want anybody going crazy before getting that first set.
Here is the Card Matching Game.
```
// Author: Matt Gowie
// Created on: 10/01/12
// Project for Web Dev 2400
$(document).ready(function(){
var cardArray =
[
'd-ace', 'd-ace', 's-ace', 's-ace', 'c-ace',
'c-ace', 'h-ace', 'h-ace', 'd-king', 'd-king',
's-king', 's-king', 'c-king', 'c-king', 'h-king',
'h-king', 'd-quen', 'd-quen', 's-quen', 's-quen',
'c-quen', 'c-quen', 'h-quen', 'h-quen', 'd-jack',
'd-jack', 's-jack', 's-jack', 'c-jack', 'c-jack',
'h-jack', 'h-jack', 'd-ten', 'd-ten', 's-ten',
's-ten', 'c-ten', 'c-ten', 'h-ten', 'h-ten',
'd-nine', 'd-nine', 's-nine', 's-nine', 'c-nine',
'c-nine', 'h-nine', 'h-nine', 'd-sev', 'd-sev',
's-sev', 's-sev', 'c-sev', 'c-sev', 'h-sev',
'h-sev', 'd-six', 'd-six', 'joker', 'joker'
];
var cardsToFlip = [];
var matches = 0;
var score = 0;
var multiplyer = 10;
fillBoard = function(){
$('.board').html('')
var rowCount = 1;
var colCount = 1;
var card = '';
for(var i = 1; i ').attr('id', "card" + i)
.addClass("row" + rowCount).addClass("col" + colCount);
var $cardContainer = $('')
.data('cardSide', 'back').append($card);
$container.html($cardContainer);
$('.board').append($container);
colCount += 1;
if(i % 10 === 0){ rowCount += 1; colCount = 1; }
}
}();
$('.container').bind('click', function(){
var cardId = $(this).attr('id');
if(cardsToFlip.length <= 1 && cardsToFlip[0] !== cardId){
flipCard(this);
cardsToFlip.push(cardId);
} else {
if(
Solution
Haven't had the time to go over all your code in detail, but here are a few ideas.
Overall, my first suggestion would be to separate the various pieces into several more functions. Keep the HTML-manipulation (building, styling, flipping cards) separate from the more abstract parts (score calculation, the card repetoire, etc.). In the same way a game of chess can be played by players with a board made of pebbles and chalk - or without a board entirely - the logic of this game is independent of its presentation.
You have several discreet steps here: Make a deck of n pairs, shuffle the deck, then make the actual HTML. Each of these steps should be a separate function. This is basically top-down design: Figure out what are the overall steps are, then figure out how to do each step.
I'd also say you could be more abstract, just in general. For one, you don't really have to care about the actual suit and value of a card - at least not in your JS; the point is just that there are n number of discreet pairs. It doesn't need to be playing cards. You have 30 pairs, but I say n because the exact number is irrelevant. There just needs to be at least 1 pair.
So far you might have something like:
The
Note that the deck is just an array of numbers at this point. The deck can be any size (that's divisible by 2 anyway), since it's not based on a pre-defined, finite list of aces, kings, queens, jacks, etc.. Now obviously, each card needs to look different, but that can be accomplished in the CSS alone. Simply give each card a class like
Of course, you could further encapsulate the deck-logic in a
Hopefully this gives you some ideas.
Overall, my first suggestion would be to separate the various pieces into several more functions. Keep the HTML-manipulation (building, styling, flipping cards) separate from the more abstract parts (score calculation, the card repetoire, etc.). In the same way a game of chess can be played by players with a board made of pebbles and chalk - or without a board entirely - the logic of this game is independent of its presentation.
You have several discreet steps here: Make a deck of n pairs, shuffle the deck, then make the actual HTML. Each of these steps should be a separate function. This is basically top-down design: Figure out what are the overall steps are, then figure out how to do each step.
I'd also say you could be more abstract, just in general. For one, you don't really have to care about the actual suit and value of a card - at least not in your JS; the point is just that there are n number of discreet pairs. It doesn't need to be playing cards. You have 30 pairs, but I say n because the exact number is irrelevant. There just needs to be at least 1 pair.
So far you might have something like:
function buildDeck(numberOfPairs) {
var i, deck = [];
for( i = 0 ; i 0 ) {
rand = Math.random() * deck.length;
shuffled.push(deck.splice(rand, 1)[0]);
}
return shuffled;
}
function buildCards(deck) {
var i, l;
for( i = 0, l = deck.length ; i < l ; i++ ) {
buildCard(deck[i]);
}
}The
buildCard function isn't implemented yet, but it would be responsible for making the HTML for a single card, and adding it to the board. That step could also be divided into more discreet steps: Build a blank card, give it its specific value, append to document, etc..Note that the deck is just an array of numbers at this point. The deck can be any size (that's divisible by 2 anyway), since it's not based on a pre-defined, finite list of aces, kings, queens, jacks, etc.. Now obviously, each card needs to look different, but that can be accomplished in the CSS alone. Simply give each card a class like
"card_" + value (kinda like you're already doing for the id attribute, except IDs must be unique, whereas there must be exactly 2 cards with the same value-class), and only deal with the "class_x → image_y" in the CSS.Of course, you could further encapsulate the deck-logic in a
Deck class and so on, and so on. But that's for another day.Hopefully this gives you some ideas.
Code Snippets
function buildDeck(numberOfPairs) {
var i, deck = [];
for( i = 0 ; i < numberOfPairs ; i++ ) {
deck = deck.push(i, i);
}
return deck;
/* re the comments, you could alternatively do this:
for( i = 0 ; i < numberOfPairs ; i++ ) {
deck = deck.push(i);
}
return deck.concat(deck);
*/
}
function shuffleDeck(deck) {
var rand, shuffled = [];
while( deck.length > 0 ) {
rand = Math.random() * deck.length;
shuffled.push(deck.splice(rand, 1)[0]);
}
return shuffled;
}
function buildCards(deck) {
var i, l;
for( i = 0, l = deck.length ; i < l ; i++ ) {
buildCard(deck[i]);
}
}Context
StackExchange Code Review Q#16386, answer score: 3
Revisions (0)
No revisions yet.