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 = '<div class="card back"></div>';
for(var i = 1; i <= 60; i++ ){
var cardPick = Math.floor(Math.random() * cardArray.length);
var cardClass = cardArray.splice(cardPick, 1)[0];
var $card = $(card).data('cardClass', cardClass);
// For debugging!
if(cardClass == 's-ace'){ $card.addClass('debug'); }
var $container = $('<div class="container">').attr('id', "card" + i)
.addClass("row" + rowCount).addClass("col" + colCount);
var $cardContainer = $('<div class="card-container">')
.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(cardsMatch()){
console.log("Cards Matched!");
$(cardsToFlip).each(function(i, e){
$("#" + e).html('')
})
matches += 1;
$('.matches').html(matches);
score += 10 * multiplyer;
$('.score').html(score)
$('.mult').html('10X')
} else {
console.log('Multiplyer: ' + multiplyer);
if(multiplyer != 1){
multiplyer -= 1;
}
$('.mult').html(multiplyer + 'X');
$(cardsToFlip).each(function(i, e){
flipCard($("#" + e));
})
}
cardsToFlip = [];
}
});
var cardsMatch = function() {
var cardOneClass = $('#' + cardsToFlip[0]).find('.card').data('cardClass');
var cardTwoClass = $('#' + cardsToFlip[1]).find('.card').data('cardClass');
if(cardOneClass === cardTwoClass){ return true; } else { return false; }
}
var flipCard = function(container) {
var classToRemove, classToAdd, flipDir;
console.log("flipCard called");
var $cardContainer = $(container).children('.card-container');
var $card = $cardContainer.children('.card');
var cardSide = $cardContainer.data('cardSide');
var cardClass = $card.data('cardClass');
if(cardSide == 'back'){
classToRemove = 'back';
classToAdd = cardClass;
flipDir = 'rl';
$cardContainer.data('cardSide', 'front');
} else {
classToRemove = 'front ' + cardClass;
classToAdd = 'back';
flipDir = 'lr';
$cardContainer.data('cardSide', 'back');
}
$card.css({ 'background-color': '#AB0000' });
$card.flip({
direction: flipDir,
onBefore: function() {
console.log("classToRemove in onBefore: " + classToRemove);
$card.removeClass(classToRemove).addClass(classToAdd);
},
onEnd: function() {
$card.css({ 'background-color': 'rgba(0, 0, 0, 0.0)' });
},
speed: '750',
color: '#AB0000'
})
}
});
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:
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]);
}
}
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.