5
\$\begingroup\$

I updated my code according to a few suggestions from my last post, so I thought I'd repost it to see what other feedback I could get. I also commented it heavily and fixed some of the functionality.

One question I do have is what is the difference between defining a function by doing

var funcName = function(){};

rather than

function funcName() {};

Which one is better or is neither better and it's just a convention to do one over the other?

You can check out the game at mattgowie.com/project-2/. And here is the source:

    // Author: Matt Gowie
// Created on: 10/01/12
// Project for Web Dev 2400
$(document).ready(function(){
  "use strict";
  var numberOfPairs   = 30;
  var cardsToFlip     = [];
  var prevCardClasses = [];
  var matches         = 0;
  var score           = 0;
  var multiplier      = 10;
  var that            = this;
  var $board = $('.board'), $winMessage = $('.win-message');

  // Check if the player won the game by matching all of the card pairs
  var checkForWin = function() {
    // Are there no more cards on the board? 
    if($('.card-container').length === 0) {
      // Then the player must have won! Hide the board and show the win message
      $board.hide();
      $winMessage.show();
    }
  };

  $('.win-message a').click(function() { 
    // How are the previous cards being reset here?
    $('.matches').html('0');
    $('.score').html('0');
    $('.mult').html('10X');
    matches    = 0;
    score      = 0;
    multiplier = 10;
    buildGameBoard(); 
  });

  // Build a deck of cards
  //
  // numberOfPairs - integer for how many pairs we need, and is 
  //   currently restricted to <= 30
  //
  // Returns an array of pairs of integers. ex: [1, 1, 2, 2, 3, 3, ... ]
  function buildDeck(numberOfPairs) {
    var deck = [];
    for(var i = 0; i < numberOfPairs; i++){
      deck.push(i,i);
    }
    return deck;
  };

  // Shuffle the given ordered deck
  // deck - An ordered array of pairs of integers
  // Returns an array of pairs of integers which are no longer ordered
  function shuffleDeck(deck) {
    var rand, shuffled = [];
    // Randomly access each pair in the given deck and add it to the resulting
    // shuffled deck
    while(deck.length > 0){
      rand = Math.random() * deck.length;
      shuffled.push(deck.splice(rand, 1)[0]);
    }
    return shuffled;
  };

  // Build the cards for the given deck and append them to the board
  // deck - a shuffled deck 
  // Returns void
  function buildCards(deck) {
    var row = 1, col = 1;
    // Loop through each card in the deck, initializing it with it's relevent info
    for( var i = 1; i <= deck.length ; i++ ) {
      buildCard(deck[i - 1], row, col, i);
      col += 1;
      // Increment the row and reset the cols for every 10 cards
      if(i % 10 === 0) { row += 1; col = 1; }
    }
  };

  // Build the html for a card and append it to the board
  // cardClass - integer which corresponds to a specific card in the deck
  // row       - integer corresponding to this cards row number
  // col       - integer corresponding to this cards col number
  // id        - integer for identifing the card for click events
  // Returns void
  function buildCard(cardClass, row, col, id) {
    var $card = $('<div class="card back"></div>');
    var $container = $('<div class="container">');
    var $cardContainer = $('<div class="card-container">');

    // Associate this card with the given class, so we know what it is when flipped
    $card.data('cardClass', 'card' + cardClass);
    if(cardClass === 's-ace'){ $card.addClass('debug'); } // For debugging!

    // Associate the cards container with the current side of the card, which
    // when the game starts is always on its back
    $cardContainer.data('cardSide', 'back');

    // Add the id, row, and col to the card container for positioning and 
    // card identification when clicking
    $container.attr('id', "card" + id).addClass("row" + row).addClass("col" + col); 

    // Put the card and containers inside one another, and then append them to the board
    $container.html($cardContainer.html($card));
    $board.append($container);
  }

  // Reset and build the game board by creating a deck, shuffling, and dealing
  function buildGameBoard() {
    // Reset the GameBoard area
    $winMessage.hide();
    $board.show().html('');

    // Initialize a new deck, shuffle, and deal.
    var deck = buildDeck(numberOfPairs);
    deck = shuffleDeck(deck);
    buildCards(deck);

    // Bind the click function for every card on the new board
    $('.container').click(function() { clickCard(this); });
  };
  buildGameBoard();

  // Flip the clicked card and if > 1 card is flip check for a match
  // container - the container of the card that was clicked
  var clickCard = function(container) {
    var cardId = $(container).attr('id');

    // Do we currently have two cards flipped? Which implies we are checking for a match
    if(cardsToFlip.length < 2) { 

      // Flip the card if we don't have two cards flipped and the card clicked 
      //   is not the same card that is already flipped
      if(cardsToFlip.length <= 1 && cardsToFlip[0] !== cardId){
        cardsToFlip.push(cardId);
        flipCard(container);
      }

      // If there are two cards flipped over check for a match 
      if(cardsToFlip.length === 2){ 
        // checkMatch is delayed 2 seconds to allow player to look at cards
        setTimeout(function(){ checkMatch(); }, 2000);
      }
    }
  }

  function incrementScoreBoard() {
    matches += 1;
    $('.matches').html(matches);
    score += 10 * multiplier;
    $('.score').html(score);
    $('.mult').html('10X');
    multiplier = 10;
  }

  // Check to see if the two cards which are flipped over are a match
  var checkMatch = function() {
    console.log('checkMatch called!');
    var cardOneClass = $('#' + cardsToFlip[0]).find('.card').data('cardClass');
    var cardTwoClass = $('#' + cardsToFlip[1]).find('.card').data('cardClass');

    if(cardOneClass === cardTwoClass){  // Do these cards match?
      // Clear the cards from the board, increment the score board, and
      //   check to see if the user won. 
      $(cardsToFlip).each(function(i, e){ $("#" + e).html(''); });
      incrementScoreBoard();
      checkForWin();
    } else {
      // No match so lower the multiplyer
      if(multiplier !== 1){ multiplier -= 1; } 
      $('.mult').html(multiplier + 'X');
      // Flip the cards onto their back side
      $(cardsToFlip).each(function(i, e){ flipCard($("#" + e)); });
    }

    // Add the cards which were just flipped to the previous card area
    $('.prev-card1').attr('class', 'card back prev-card1 ' + prevCardClasses[0]);
    $('.prev-card2').attr('class', 'card back prev-card2 ' + prevCardClasses[1]);
    prevCardClasses = [];
    cardsToFlip = [];
  };

  // Flip the card container in the given container. 
  var flipCard = function(container) {
    var classToRemove, classToAdd, flipDir;
    var $cardContainer = $(container).children('.card-container');
    var $card = $cardContainer.children('.card');
    var cardSide = $cardContainer.data('cardSide');
    var cardClass = $card.data('cardClass');

    if(cardSide === 'back'){ // Is this card face down?
      classToRemove = 'back';
      classToAdd = cardClass;
      flipDir = 'rl';
      $cardContainer.data('cardSide', 'front');
    } else { // This card is face up.
      classToRemove = 'front ' + cardClass;
      classToAdd = 'back';
      flipDir = 'lr';
      $cardContainer.data('cardSide', 'back');
      prevCardClasses.push(cardClass);
    }

    // Use jquery.flip plugin to flip the card up or down depending on vars
    //   set previously 
    $card.css("background-color", "#AB0000" );
    $card.flip({
      direction: flipDir,
      onBefore: function() {
        $card.removeClass(classToRemove).addClass(classToAdd);
      },
      onEnd: function() {
        $card.css({ 'background-color': 'rgba(0, 0, 0, 0.0)' });
      },
      speed: '200',
      color: '#AB0000'
    });
  };

});
\$\endgroup\$
  • 5
    \$\begingroup\$ You can look at this post to see the [difference between a function expression vs declaration in Javascript][1] [1]: stackoverflow.com/questions/1013385/… \$\endgroup\$ – Kinjal Oct 15 '12 at 1:10
  • 1
    \$\begingroup\$ Pretty nice update. Perhaps the checkMatch function is still doing a little too much (i.e. not just checking for a match, but also calculating the multiplier, flipping cards, updating the prev-cards stuff). That's the kind of stuff that could be broken into separate functions. But I'll let someone else take a closer look this time :) \$\endgroup\$ – Flambino Oct 15 '12 at 10:25
  • \$\begingroup\$ The link you provided is dead, can you provide a link to the previous review ? \$\endgroup\$ – konijn Dec 23 '13 at 16:07
2
\$\begingroup\$

I like your code

  • "Use Strict" within a IFFE
  • Good level of comments
  • Correct casing and decent naming
  • Nice size of methods

Some very minor stuff

  • JsHint says you never use that ( line 12 ) nor i declard on line 163 and line 171
  • function(){} should not have a semicolon after the closing curly brace (buildCards)
  • var f = function(){} should have a semicolon after the closing curly brace (clickCard)
  • You have a magical constant in buildCards which could be extracted and named.
  • You have a magical constant in incrementScoreBoard which could be extracted and named.
  • You should take out all debugging related code in your final version

As for var funcName = function(){}; versus function funcName() {};, I always thought the second form was silly. Until I read this:

// Don't do this:
function getData() { }

// Do this instead:
var getData = function() { };

Whenever JR says something about JavaScript, one should pay attention. Since 2011 though we have come to the practical conclusion that anonymous functions in the callstack are a major PITA. which means that if we stick to 'Do this instead' we ought to write.

var getData = function getData(){};

And that just looks wrong ( not DRY ), and should be avoided in my book.

\$\endgroup\$
  • \$\begingroup\$ Hey, I posted this a long time ago and it's long since been forgotten, but thanks for the feedback. One interesting thing you brought up that I've just recently started changing about my code is the var funcName = function(){}; versus function funcName() {}; convention. After this project I start doing the former, but I recently switched to the latter. I switched because of the function.name field which isn't populated when using the anon function route. Using your last example ('var getData..') would fix this issue, but I agree with you on the ugliness. What do you do exactly? \$\endgroup\$ – Gowie47 Jan 7 '14 at 0:31

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.