A Game Of Simon

Question

This is one of the projects from freeCodeCamp. It's a game of Simon (a memory game). I would like a review on my code.

var generatedPattern = [];
var playerPattern = [];
var patternLength = 1;
var redRef = 1;
var greenRef = 2;
var blueRef = 3;
var yellowRef = 4;

function generateNextPatternValue() {

  if (patternLength == 20) {
    gameReset();
  } else {
    var next = getRandomNum(1, 4);
    if (next === generatedPattern[generatedPattern.length - 1] && next == 4)
      next--;
    else if (next === generatedPattern[generatedPattern.length - 1])
      next++;

    generatedPattern.push(next);
    animateGeneratedPattern();
  }
}

function getRandomNum(min, max) {
  return Math.floor(Math.random() * (max - min + 1)) + min;
}

function animateRed(lightup) {

  if (lightup) {
    var audio = new Audio("https://s3.amazonaws.com/freecodecamp/simonSound1.mp3");
    audio.play();
  }
  $("#1").toggleClass("animateRed");

}

function animateGreen(lightup) {

  if (lightup) {
    var audio = new Audio("https://s3.amazonaws.com/freecodecamp/simonSound2.mp3");
    audio.play();
  }
  $("#2").toggleClass("animateGreen");

}

function animateBlue(lightup) {

  if (lightup) {
    var audio = new Audio("https://s3.amazonaws.com/freecodecamp/simonSound3.mp3");
    audio.play();
  }
  $("#3").toggleClass("animateBlue");

}

function animateYellow(lightup) {

  if (lightup) {
    var audio = new Audio("https://s3.amazonaws.com/freecodecamp/simonSound4.mp3");
    audio.play();
  }
  $("#4").toggleClass("animateYellow");

}

function animateGeneratedPattern() {

  var i = 0;

  function animateNextPattern(lightup) {
    if (!generatedPattern || generatedPattern.length === i) {
      return;
    }

    switch (generatedPattern[i]) {
      case 1:
        animateRed(lightup);
        break;
      case 2:
        animateGreen(lightup);
        break;
      case 3:
        animateBlue(lightup);
        break;
      case 4:
        animateYellow(lightup);
        break;
    }
    if (lightup) {
      // Long delay before turning light off
      setTimeout(function() {
        animateNextPattern(false);
      }, 1000);
    } else {
      // Small delay before turning on next light
      i++;
      setTimeout(function() {
        animateNextPattern(true);
      }, 100);
    }

  }
  animateNextPattern(true);

}

function gameReset() {

  if (patternLength == 20)
    alert("You Have Won");
  document.getElementById("start").disabled = false;
  playerPattern = [];
  generatedPattern = [];
  patternLength = 1;
  updateLength();
  restoreColors();
}

function restoreColors() {

  $("#1").removeClass("animateRed");
  $("#2").removeClass("animateGreen");
  $("#3").removeClass("animateBlue");
  $("#4").removeClass("animateYellow");

}

function updateLength() {

  $("#length").empty();
  var txtNode = document.createTextNode(patternLength);
  document.getElementById("length").appendChild(txtNode);

}

function animateScreen(turnOn) {

  if (turnOn) {
    var audio = new Audio("http://www.sounds.beachware.com/2illionzayp3may/illwavul/CLNKBEEP.mp3");
    audio.play();
  }
  $("body").toggleClass("wrongSeries");

}

function wrongChoice() {

  animateScreen(true);
  setTimeout(function() {
    animateScreen(false);
  }, 100);

}

function unbind() {

  $("#1").unbind("click");
  $("#2").unbind("click");
  $("#3").unbind("click");
  $("#4").unbind("click");

}

function bind() {

  $("#1").bind("click", function() {
    if (document.getElementById("start").disabled) {
      playerPattern.push(1);
      if (checkSeries()) {
        animateRed(true);
        setTimeout(function() {
          animateRed(false);
        }, 500);
      }
    }
  });

  $("#2").bind("click", function() {
    if (document.getElementById("start").disabled) {
      playerPattern.push(2);
      if (checkSeries()) {
        animateGreen(true);
        setTimeout(function() {
          animateGreen(false);
        }, 500);
      }
    }
  });

  $("#3").bind("click", function() {
    if (document.getElementById("start").disabled) {
      playerPattern.push(3);
      if (checkSeries()) {
        animateBlue(true);
        setTimeout(function() {
          animateBlue(false);
        }, 500);
      }
    }

  });

  $("#4").bind("click", function() {
    if (document.getElementById("start").disabled) {
      playerPattern.push(4);
      if (checkSeries()) {
        animateYellow(true);
        setTimeout(function() {
          animateYellow(false);
        }, 500);
      }
    }
  });

}

function checkSeries() {

  if (playerPattern[playerPattern.length - 1] !== generatedPattern[playerPattern.length - 1]) {
    if (document.getElementById("strict").checked) {
      wrongChoice();
      gameReset();
      document.getElementById("start").disabled = true;
      setTimeout(generateNextPatternValue, 1500);
    } else {
      unbind();
      wrongChoice();
      playerPattern = [];
      setTimeout(animateGeneratedPattern, 1500);
      setTimeout(bind, 1500 * patternLength);
    }
    return false;
  } else if (playerPattern.length == patternLength) {
    unbind();
    patternLength++;
    updateLength();
    playerPattern = [];
    setTimeout(generateNextPatternValue, 1000);
    setTimeout(bind, 1500 * patternLength);
    return true;
  }

  return true;
}

window.addEventListener("load", updateLength);
window.addEventListener("load", bind);

$("#start").on("click", function() {

  document.getElementById("start").disabled = true;
  generateNextPatternValue();

});

$("#reset").on("click", gameReset);

Full code here.

Answer 1

Duplicate Code

A lot of your code is really similar to other parts. The duplication is generally found when working with your colors. For example, take a look at your animiateColor functions. The only differences between them are the audio file, the html element, and the CSS class. You could shorten your code while making it easier to edit by merging the similar parts together:

function animiateColor(color) { // your lightup boolean controls the sound?
  color.audio.play();
  color.element.toggleClass(color.animateClass);
}

function stopAnimation(color) {
  color.element.toggleClass(color.animateClass);
}

animiateColor({
  audio: new Audio("http url red"),
  element: $("#1"),
  animateClass: "animateRed"
});

You should consider making an array of all your colors instead of giving each their own variable. This allows you to iterate through the colors as opposed to explicitly executing functions on each one:

var colors = [
  {
    audio: new Audio("http red"), // this also caches the audio file, more on that later
    element: $("#1"),
    animateClass: "animateRed"
  },
  {
    audio: new Audio("http green"),
    element: $("#1"),
    animateClass: "animateGreen"
  },
  ...
];

With the array you can iterate through each element using a for loop or the Array.forEach method:

function unbind() {
  colors.forEach(function (color) {
    color.element.unbind("click");
  });
}

function bind() {
  colors.forEach(function (color, index) {
    color.element.bind("click", function() {
      if (document.getElementById("start").disabled) {
        playerPattern.push(index);

        if (checkSeries()) {
          animiateColor(color);
          setTimeout(function () {
            stopAnimation(color);
          }, 500);
        }
      }
   });
}

See how much shorter this has become? Now if you want to change how any of these methods work, you only need to make that change in one place. Additionally, if you ever decide to add another color to the game, you'd only need to add it to the array.

Randomly Selecting a Color

Your algorithm for randomly selecting the next color is weighted heavily for the color one greater than the last one chosen (except yellow which is 1 less). For example, if blue was the last to flash, there's a 50% chance the next chosen color is yellow.

Proof: if blue(3) was the last chosen, then the random number from 1 to 4 has the following cases:

1 => Red, 2 => Green, 3 => Yellow, 4 => Yellow

Since each number has a 25% chance of being chosen, Yellow has a 50% chance of being the next color.

If you decide to use the array of colors, you could randomly select one without repetition like so:

function getNextIndex(arrayLength, lastIndex) {
  var nextIndex = lastIndex + getRandomNum(1, arrayLength - 1);
  return nextIndex % 4;
}

How it works: The three other indices are either 1, 2, or 3 away from the lastIndex--that is relative to mod 4 (think of them as cycling around e.g. yellow + 1 = red)

Better User Interaction

I tried out your codepen and had a few problems with the user interface. The flashing was hard to see for some colors, and there was an inconsistent delay between clicking and hearing the audio play.

I think you ought to make the colors flash brighter:

.animateRed {
  background: #FFCCCC;
}
.animateGreen {
  background: #CCFFCC;
}
.animateBlue {
  background: #CCCCFF;
}
.animateYellow {
  background: #FFFFCC;
}

And to eliminate the sound delay, you should cache your sounds, so your code doesn't load a new Audio object for each animation:

var sounds = {
  red: new Audio("http link"),
  blue: new Audio(...),
  ....
};

sounds.red.play();
sounds.red.play();
sounds.red.play(); // each call uses the same audio

This will make your game more responsive and enjoyable to play.

---

Your project looks like it's turning out pretty well! Keep up the hard work.