Simple color clicker game

Question

My friend recently made a color clicker game, and I tried cleaning up the code a bit. I want to know how I can improve the functions and passing/editing of global variables.

There are a lot of functions and global variables. To keep the global variables 'local', I wrapped the code in an anonymous function. Would it be better to have the functions pass the variables around instead of having them as global?

I'd also like to know how I can improve the syntax and use ES6 to replace snippets. Thanks!

(function() {
let numSquares = 6;
let colors = [];
// Get dom elements
let h1 = document.querySelector("h1");
let squares = document.getElementsByClassName('square');
let colorDisplay = document.getElementById("colorDisplay");
let messageDisplay = document.getElementById("message");
let easyBtn = document.getElementById("easyBtn");
let hardBtn = document.getElementById("hardBtn");
initGame();
// Generates array of random RGB values, and picks one
function initGame() {
 // Setup array of colors and which to use for the round
 initColors();
 // Setup squares and event listeners
 initSquares();
 // Setup buttons
 initOptions();
 initReset();
}
// Make an array of randomly generated colors depending on game mode
function initColors() {
 colors = generateRandomColors(numSquares);
 winningColor = pickWinningColor();
 colorDisplay.textContent = winningColor;
 if(numSquares === 6) {
 for(let i = 0; i < squares.length; i++ ) { 
 squares[i].style.background = colors[i];
 squares[i].style.display = "block"; 
 } 
 } 
 else if(numSquares === 3) {
 for(let i = 0; i < squares.length; i++ ) {
 if(colors[i]) {
 squares[i].style.background = colors[i];
 } else {
 squares[i].style.display = "none";
 }
 }
 }
}
// Setup squares colors and listeners
function initSquares() {
 for(let i = 0; i < squares.length; i++) {
 squares[i].style.background = colors[i];
 squares[i].addEventListener("click", function(){
 let clickedColor = this.style.backgroundColor;
 if(clickedColor === winningColor) {
 messageDisplay.textContent = "Correct!";
 changeColors(winningColor);
 h1.style.background = clickedColor;
 }
 else {
 this.style.background = "#232323";
 messageDisplay.textContent = "Try Again";
 } 
 });
 }
}
// Setup option buttons
function initOptions() {
 let buttons = document.getElementsByClassName('option');
 for(let i = 0; i < buttons.length; i++) {
 buttons[i].addEventListener('click', function() {
 if(this.getAttribute('id') === "hardBtn") {
 hardBtn.classList.add('selected');
 easyBtn.classList.remove('selected');
 numSquares = 6;
 initColors();
 } 
 else if(this.getAttribute('id') === "easyBtn") {
 hardBtn.classList.remove('selected');
 easyBtn.classList.add('selected');
 numSquares = 3;
 initColors();
 }
 });
 }
}
// Setup reset button
function initReset() {
 let resetBtn = document.getElementById('reset');
 resetBtn.addEventListener('click', function() {
 initColors(numSquares);
 h1.style.background = "steelblue";
 messageDisplay.textContent = " ";
 this.textContent = "reset colors";
 });
}
// Change square colors
function changeColors(color) {
 for(let i = 0; i < squares.length; i++) {
 squares[i].style.background = color;
 }
}
// Pick the color player needs to guess
function pickWinningColor() {
 let random = Math.floor(Math.random() * colors.length);
 return colors[random];
}
// Generate random RGB values for each square
function generateRandomColors(num) {
 let colors = [];
 for(let i = 0; i < num; i++) {
 colors.push(randomizeNewRGB());
 }
 return colors;
}
// Return new RGB value tuple
function randomizeNewRGB() { 
 let [r, g, b] = [0, 0, 0].map(oldColorVal => (Math.floor(Math.random() * (256 - oldColorVal)) + oldColorVal) % 256);
 return `rgb(${r}, ${g}, ${b})`;
}
})();

body {
 background-color: #232323;
 margin: 0;
 font-family: "Montserrat", "Avenir"
}
.square {
 width:30%;
 background: purple;
 padding-bottom: 30%;
 float:left;
 margin:1.66%;
 border-radius: 15%;
 transition: background 0.5s;
}
.square:hover {
 cursor: pointer;
}
.container {
 max-width: 600px;
 margin: 15px auto;
}
h1 {
 color:white;
 line-height: 1.1;
 font-weight: normal;
 text-transform: uppercase;
 text-align: center;
 background-color: steelblue;
 margin: 0;
 padding: 20px 0;
}
#colorDisplay {
 font-size: 200%;
}
#stripe {
 background: white;
 height: 30px;
 text-align: center;
 color: black;
}
.selected {
 color: white;
 background: steelblue;
}
button {
 border: none;
 background: none;
 text-transform: uppercase;
 font-family: "Montserrat", "Avenir";
 height: 100%;
 font-weight: 700;
 color: steelblue;
 letter-spacing: 1px;
 font-size: inherit;
 transition: all 0.3s;
}
button:hover {
 cursor: pointer;
 color: white;
 background: steelblue;
}
#message {
 display: inline-block;
 width: 20%;
}

<!DOCTYPE html>
<html lang="en">
<head>
 <meta charset="UTF-8">
 <meta name="viewport" content="width=device-width, initial-scale=1.0">
 <meta http-equiv="X-UA-Compatible" content="ie=edge">
 <meta http-equiv="content-type" content="text/html; charset=utf-8" />
 <title>Color Game</title>
 <link rel="stylesheet" href="../../public/css/style.css">
</head>
<body>
 <h1>The Josh <br> <span id="colorDisplay"> RGB </span><br>Game</h1>
<div id="stripe">
 <button id="reset">Reset Colors</button>
 <span id="message"></span>
 <button class="option" id="easyBtn" >Easy</button>
 <button class="option selected" id="hardBtn" class="selected">Hard</button>
</div>
<div class="container">
 <div class="square"></div>
 <div class="square"></div>
 <div class="square"></div>
 <div class="square"></div>
 <div class="square"></div>
 <div class="square"></div>
</div>
 <script src="../../public/js/script.js"></script>
</body>
</html>

Answer 1

Feedback

• Avoided Global vars Nice job using let inside an IIFE to prevent the scope of variables from becoming global.
• Caching DOM Elements good job storing references to DOM elements in variables.
• Strict Comparison operators The code uses strict comparison operators (e.g. if(numSquares === 6) {) which avoids useless type conversions.

Suggestions

• Use const Any variable that doesn't get re-assigned can be declared with const to avoid accidental re-assignment

• for..of loops some for loops could be transformed using for...of loops to simplify accessing each element in the array. For instance,

  function changeColors(color) {
   for(let i = 0; i < squares.length; i++) {
   squares[i].style.background = color;
   }
  } 
  

  could be simplified to something like the following:

  function changeColors(color) {
   for(const square of squares) {
   square.style.background = color;
   }
  }
  

• Excessive CSS The rule font-family: "Montserrat", "Avenir"; under button seems redundant, since that cascades from the same rule under body