JavaScript Memory Game with Pictures

Question

So, I have a memory game written, but was wondering how I could turn these letters in the cardSet into images (I have a set of images in an images folder within the folder containing this code, called cat.png, cow.png, etc). Here's the js:

$(document).ready(function () {
  var cardSet = ["A", "A", "B", "B", "C", "C", "D", "D",
                   "E", "E", "F", "F", "G", "G", "H", "H"];
  randomize(cardSet);
  load(cardSet);

  var tries = 0;
  var timer;
  var pair = 0;
  var $box1 = undefined;
  var $box2 = undefined;

  $("button").click(function () {
      if ($(this).attr("class") == "hidden") {
          if ($box1 == undefined) {
              $box1 = $(this);
              $box1.removeClass("hidden").addClass("show");

              timer = setTimeout(function () {

              if ($box2 == undefined) {
                $box1.removeClass("show").addClass("hidden");
                $box1 = undefined;
                numberOfClicks += 1;
              } else if ($box1.text() != $box2.text()) {
                $box1.removeClass("show").addClass("hidden");
                $box1 = undefined;
                $box2.removeClass("show").addClass("hidden");
                $box2 = undefined;
                tries += 1;
              }
            }, 3000);

        } else if ($box2 == undefined) {
            $box2 = $(this);
            $box2.removeClass("hidden").addClass("show");

            if ($box1.text() == $box2.text()) {
                pair += 1;
                tries += 1;
                $box1 = undefined;
                $box2 = undefined;
            };

        };
    };

    if (pair == 8) {
        alert("Tries: " + tries);
    }

 });

});

function load(cardSet) {
  var index = 0;
  for (i = 0; i < 4; i++) {
      for (j = 0; j < 4; j++) {
        $("#b" + i + j).text(cardSet[index]);
        index++;
      }
  }
}

function randomize(array) {
  var currentIndex = array.length, temporaryValue, randomIndex ;
  while (0 !== currentIndex) {
    randomIndex = Math.floor(Math.random() * currentIndex);
    currentIndex -= 1;
    temporaryValue = array[currentIndex];
    array[currentIndex] = array[randomIndex];
    array[randomIndex] = temporaryValue;
  }
  return array;
}

table {
  margin-left: auto;
  margin-right: auto;
  border-collapse: collapse;
  border: 1px solid;
}

table td {
  border: 1px solid;
  height: 100px;
  width: 100px;
  text-align: center;
}

button {
  border: 0px solid;  
  height: 100px;
  width: 100px;
  text-align: center;
  font-size: 150%;
  font-weight: 300;
  font-family: "Questrial", sans-serif; 
  color: #000000;
  background-color: #FFFFFF;
}

.hidden {
  opacity: 0;
}

.show {
 opacity: 1;  
}

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<table width = "500px" height = "500px">
  <tr>
      <td><button id="b00" class="hidden"></button></td>
      <td><button id="b01" class="hidden"></button></td>
      <td><button id="b02" class="hidden"></button></td>
      <td><button id="b03" class="hidden"></button></td>
  </tr>
  <tr>
      <td><button id="b10" class="hidden"></button></td>
      <td><button id="b11" class="hidden"></button></td>
      <td><button id="b12" class="hidden"></button></td>
      <td><button id="b13" class="hidden"></button></td>
  </tr>
  <tr>
      <td><button id="b20" class="hidden"></button></td>
      <td><button id="b21" class="hidden"></button></td>
      <td><button id="b22" class="hidden"></button></td>
      <td><button id="b23" class="hidden"></button></td>
  </tr>
  <tr>
      <td><button id="b30" class="hidden"></button></td>
      <td><button id="b31" class="hidden"></button></td>
      <td><button id="b32" class="hidden"></button></td>
      <td><button id="b33" class="hidden"></button></td>
  </tr>
</table>

I apologize for the messy code... Fairly new to javascript. Any advice is accepted though!

Answer 1

My two cents,

you could extract the

.removeClass("hidden").addClass("show");

and

.removeClass("show").addClass("hidden");

to two new methods

function showBox($element) {
    $element.removeClass("hidden").addClass("show");
}

function hideBox($element) {
    $element.removeClass("show").addClass("hidden");
}

and use the methods in it's place for cleaner code.

Answer 2

Game design

Don't rush me

Why do the letters disappear after a few a seconds? What if I need time to think?

To me, this seems like a "hacky way" to hide the cards after two were chosen. Also, this even leads to a problem that occurs after a pair is found: the space I click on next shortly disappears, even though I just clicked it.

After a pair is found, you should clear the timeout with clearTimeout so the next spaces that are clicked do not disappear.

---

Hey, I wanna see my score!

I see this in your code:

if (pair == 8) {
    alert("Tries: " + tries);
}

But, when I match all the squares, I don't get an alert showing my tries (and it's not just because I have popups blocked).

---

Design

Slicing the methods right down the middle

$box1.removeClass("show").addClass("hidden");

You seem to be repeating this pattern of removeClass and then addClass a lot. In jQuery, this is a method called toggleClass that will add the class if it does not exist in the element, and will remove it if it does.

To simplify your code and cut the method calls in half, you should set the styles of hidden or shown to be the default styles of the buttons. Then, when you want to apply the other class, use toggleClass. The applied class will override the default styles.

This should speed up your code.

---

An ID for you, an ID for you, oh; an ID for you, too.

An ID for each button? That's a really bad design.

A better design would be to identify them by something that they all share and is unique to these buttons. In this case, since these are the only buttons on your page, you can identify them with their tag name:

$("button");

---

Nitpickery

Do the math

function load(cardSet) {
  var index = 0;
  for (i = 0; i < 4; i++) {
      for (j = 0; j < 4; j++) {
        $("#b" + i + j).text(cardSet[index]);
        index++;
      }
  }
}

Having that extra index variable is unnecessary. You can just use the variables you already have: i and j and the number 4.

$("#b" + i + j).text(cardSet[i * 4 + j]);

---

There's a bump in your indentation

You are being inconsistent with your indentation. In some places, you are indenting two spaces. In others, four. I recommend that you stick with one (I like four).

Defining the undefined

var $box1 = undefined;

In JavaScript, it's sorta bad practice to set a variable to undefined, because that is also the value you get for something that has not yet been defined or has no definition.

This can make your code confusing. Instead, I recommend that you use null. This shows that the value had been defined at some point in time, just there is not supposed to be any value in it.

---

Weakqual

In JavaScript, these are bad.

==

or

!=

While they are still comparing two values, these both will cast the second value to be the same type as the first value, which can result in conditions that really shouldn't pass.

You should always use

===

or

!==

---

What is wrong in this picture?

numberOfClicks += 1;

Whoa! Where did that come from? All of a sudden increasing a value that was not previously declared anywhere? That's not good.

I have no idea what you are trying to do here, but it looks like you meant to use the variable tries.