Custom Connect Four game on CodePen using JavaScript

Question

Can this code be shortened, optimized or remove any unnecessary parts. Just curious if this is the right way to achieve the given result in this CodePen example: CodePen Connect Four

Here is my code:

 // Debug & fix this Script.. document.addEventListener('DOMContentLoaded',()=>{ const displayCurrentPlayer = document.querySelector('#current-player') const squares = document.querySelectorAll('.grid div') const resetBtn = document.querySelector('#Reset') const result = document.querySelector('#result') let currentPlayer = 1 for(var i = 0, len = squares.length; i < len; i++) (function(index){ // add an onclick to each square in the grid squares[i].onclick = function(){ // if the square below your current square is taken, you can go ontop of it. if(squares[index + 7].classList.contains('taken')){ if(!squares[index].classList.contains('taken')){ if(currentPlayer === 1){ squares[index].classList.add('taken') squares[index].classList.add('player-one') // Change the player currentPlayer = 2 displayCurrentPlayer.innerHTML = currentPlayer // displayCurrentPlayer.style.background = 'blue'; }else if(currentPlayer === 2){ squares[index].classList.add('taken') squares[index].classList.add('player-two') // Change the player currentPlayer = 1 displayCurrentPlayer.innerHTML = currentPlayer // displayCurrentPlayer.style.backgroundImage = `linear-gradient(-33deg,blue,green)!important`; } } } else alert('Cant Go Here') } resetBtn.addEventListener('mousedown',(e) => { if(squares[index + 7].classList.contains('taken')){ squares[index].classList.remove('taken') squares[index].classList.remove('player-one') squares[index].classList.remove('player-two') displayCurrentPlayer.innerHTML = 1 } }); })(i) //check the board for a win or lose function checkBoard() { //make const that shows all winning arrays const winningArrays = [ [0, 1, 2, 3], [41, 40, 39, 38], [7, 8, 9, 10], [34, 33, 32, 31], [14, 15, 16, 17], [27, 26, 25, 24], [21, 22, 23, 24], [20, 19, 18, 17], [28, 29, 30, 31], [13, 12, 11, 10], [35, 36, 37, 38], [6, 5, 4, 3], [0, 7, 14, 21], [41, 34, 27, 20], [1, 8, 15, 22], [40, 33, 26, 19], [2, 9, 16, 23], [39, 32, 25, 18], [3, 10, 17, 24], [38, 31, 24, 17], [4, 11, 18, 25], [37, 30, 23, 16], [5, 12, 19, 26], [36, 29, 22, 15], [6, 13, 20, 27], [35, 28, 21, 14], [0, 8, 16, 24], [41, 33, 25, 17], [7, 15, 23, 31], [34, 26, 18, 10], [14, 22, 30, 38], [27, 19, 11, 3], [35, 29, 23, 17], [6, 12, 18, 24], [28, 22, 16, 10], [13, 19, 25, 31], [21, 15, 9, 3], [20, 26, 32, 38], [36, 30, 24, 18], [5, 11, 17, 23], [37, 31, 25, 19], [4, 10, 16, 22], [2, 10, 18, 26], [39, 31, 23, 15], [1, 9, 17, 25], [40, 32, 24, 16], [9, 7, 25, 33], [8, 16, 24, 32], [11, 7, 23, 29], [12, 18, 24, 30], [1, 2, 3, 4], [5, 4, 3, 2], [8, 9, 10, 11], [12, 11, 10, 9], [15, 16, 17, 18], [19, 18, 17, 16], [22, 23, 24, 25], [26, 25, 24, 23], [29, 30, 31, 32], [33, 32, 31, 30], [36, 37, 38, 39], [40, 39, 38, 37], [7, 14, 21, 28], [8, 15, 22, 29], [9, 16, 23, 30], [10, 17, 24, 31], [11, 18, 25, 32], [12, 19, 26, 33], [13, 20, 27, 34] ]; //now take the 4 values in earch winningArray & plug them into the squares values for(let y = 0; y < winningArrays.length; y++) { const square1 = squares[winningArrays[y][0]]; const square2 = squares[winningArrays[y][1]]; const square3 = squares[winningArrays[y][2]]; const square4 = squares[winningArrays[y][3]]; //now check those arrays to see if they all have the class of player-one if(square1.classList.contains('player-one') && square2.classList.contains('player-one') && square3.classList.contains('player-one') && square4.classList.contains('player-one')) { //if they do, player-one is passed as the winner result.innerHTML = 'Player one wins!' //remove ability to change result } //now check to see if they all have the classname player two else if (square1.classList.contains('player-two') && square2.classList.contains('player-two') && square3.classList.contains('player-two') && square4.classList.contains('player-two')) { //if they do, player-two is passed as the winner as well as the chip positions result.innerHTML = 'Player two wins!' } } } //add an event listener to each square that will trigger the checkBoard function on click squares.forEach(square => square.addEventListener('click', checkBoard)) });

*{ box-sizing:border-box; text-align:center; } @font-face{ font-family:BlackChancery; src:url(../blkChancery.ttf) format('truetype'); } body{ background:teal; font-family:BlackChancery; font-size:1.17rem; /* Nice golden yellow color 252, 186, 3 */ } .grid{ margin:auto; margin-top:calc(2vh + 120px); border:solid 1px black; overflow:hidden; transform:scale(2.4); /* translate(-50%,-50%) */ display:flex;align-content:center; justify-content: center; flex-wrap:wrap;background-image:linear-gradient(-33deg,#384e65,#2e2e2e); height:124px; width:144px; } .grid div{ height:20px; width:20px; border-radius:50%; background:white; border:solid 1.8px black; filter:hue-rotate(0deg) blur(0.44px); transition:all 980ms ease-in-out; } .base{ /* Base set of Circles which are not used by players. */ display:none; } .player-one{ background-image:linear-gradient(-33deg,red,rgb(186,4,119),rgb(186,4,119))!important; border-radius:10px; filter:hue-rotate(0deg) blur(0.34px); transform:rotate(0deg); transition:all 980ms ease-in-out; } .player-two{ background-image:linear-gradient(-33deg,blue,rgb(4,186,156),rgb(4,186,156))!important; border-radius:10px; filter:hue-rotate(0deg) blur(0.34px); transform:rotate(0deg); transition:all 980ms ease-in-out; } .player-one:hover{ filter:hue-rotate(-49deg) blur(0.34px); transform:rotate(54deg); transition:all 980ms ease-in-out; } .player-two:hover{ filter:hue-rotate(47deg) blur(0.34px); transform:rotate(54deg); transition:all 980ms ease-in-out; } #current-player{ position:relative; background-image:linear-gradient(-33deg,red,rgb(186,4,119),rgb(186,4,119))!important; filter:blur(0.588px); text-shadow:1px 1px 1.7px white; border:solid 2.98px black; width:148px !important; height:48px !important; font-size:1.94rem; border-radius:50%; padding:8px 28px; margin-top:8px; top:8px; } #Reset{ margin-top:calc(1.8vh + 78px); padding:4px 8px 4px 8px; border-radius:7px; background:rgb(67, 240, 211); font-size:1.24rem; } /* position:absolute; top:50vh; left:50vw; */

 <h1>Custom Connect Four<br>2 player Game</h1> <h3>The Current Player<br>is: Player <br><span id="current-player" width="50">1</span></h3> <h3 id="result"></h3> <div class="grid"> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div><div></div><div></div><div></div> <div></div> <div></div> <div class="taken base"></div> <div class="taken base"></div> <div class="taken base"></div> <div class="taken base"></div> <div class="taken base"></div> <div class="taken base"></div> <div class="taken base"></div> </div><br> <button id="Reset">Reset</button> <script src="https://cdnjs.cloudflare.com/ajax/libs/animejs/3.2.0/anime.min.js" integrity="sha256-hBMojZuWKocCflyaG8T19KBq9OlTlK39CTxb8AUWKhY=" crossorigin="anonymous"></script>

Are there any errors that I've not spotted? How could it be improved? Styling could be a lot better I guess, but any other suggestions apart from that?

In the codepen example i have switched out all the let & const statements & created a helper function for document.querySelector & selectorAll..

Answer 1

Use the defer attribute on the script tag instead of having a DOMContentLoaded listener - it'll cut down on a level of indentation throughout your entire script.

Use let instead of an IIFE to solve the closure-inside-loops issue - let is block-scoped to each iteration inside the loop out-of-the box, so there's no need for an IIFE - it significantly reduces the noisyness of the code. If you need to support IE11, then like always, the tried-and-true professional solution is to write code in the latest version of the language, keeping the source code as readable and elegant as possible, and then use Babel to transpile down to ES5 automatically for production. (Don't transpile to ES5 manually!)

Another option would be to consider using event delegation instead of adding a listener to each cell: watch for clicks on the whole .grid, and when a cell is clicked, get the target child's index from the collection of children inside the click listener.

Save the clicked square in a variable instead of repeating squares[index] lots of times - write DRY code.

base elements are weird - you have 7 <div class="taken base"></div> elements at the end presumably in order for the squares[index + 7].classList.contains('taken') test to work. These taken base elements are always hidden - they exist solely for this logic. Consider removing them entirely, and instead checking to see if squares[index + 7] exists first:

const belowSquare = squares[index + 7]; if(!belowSquare || belowSquare.classList.contains('taken')){ 

Enter short branches first so you can return early without requiring many }s at the end of a long block - it makes the logic easier to follow when a reader of the code doesn't have to keep track of a condition that was used 20 lines ago.

if (belowSquare && !belowSquare.classList.contains('taken')) { alert('Cant Go Here') return; } // rest of the code 

Avoid alert and its siblings confirm and prompt - they're not user-friendly, since they block the browser; when one of those dialogs is active, other JavaScript and page painting will not occur, and the page will seem completely unresponsive until the dialog is dismissed. Better to make a proper modal in the DOM and insert text into it instead.

Grammar Proper spelling and grammer really helps improve the presentation of an app. Use "Can't go here".

DRY circle toggling Rather than two separate branches for each possible player, make the player-# class use a number for the player rather than words, so that setting the active player just requires concatenation with the currentPlayer:

square.classList.add('taken'); square.classList.add('player-' + currentPlayer); currentPlayer = currentPlayer === 1 ? 2 : 1; 

Only use innerHTML when deliberately inserting HTML markup - otherwise, textContent is faster, safer, and more appropriate.

Don't add the same listener to an element lots of times in a loop with resetBtn, since the logic to carry out is the same - instead, add a single listener to the button, and inside the listener, iterate over all elements necessary. Also, there's no need to declare parameters you don't use: (e) => can be () =>. Lastly, to reset all the class names of an element, you can assign the empty string to its className.

resetBtn.addEventListener('click', () => { for (const square of squares) { square.className = ''; } displayCurrentPlayer.textContent = 1; }); 

Use precise, descriptive function names to make the code more self-documenting, and to avoid the need for comments. This:

//check the board for a win or lose function checkBoard() { 

can be

checkBoardForWinner() { 

Also, rather than adding a click listener to each square that calls this, call this inside the listener already on each square.

winningArrays is WET and buggy There are many 4-in-a-row combinations it doesn't pick up on. For example:

A better approach would be to encode the 4-in-a-row logic dynamically based in the indicies. Iterate over all cells, and check whether the northeast, east, southeast, or south have 3 of the same squares in a row. See checkBoardForWinner below:

const displayCurrentPlayer = document.querySelector('#current-player'); const squares = document.querySelectorAll('.grid div'); const resetBtn = document.querySelector('#Reset'); const result = document.querySelector('#result'); let currentPlayer = 1; for (let i = 0, len = squares.length; i < len; i++) { const square = squares[i]; square.onclick = () => { // if the square below your current square is taken, you can go on top of it. const belowSquare = squares[i + 7]; if (belowSquare && !belowSquare.classList.contains('taken')) { alert("Can't go here"); return; } square.classList.add('taken'); square.classList.add('player-' + currentPlayer); currentPlayer = currentPlayer === 1 ? 2 : 1; displayCurrentPlayer.textContent = currentPlayer; checkBoardForWinner(); }; } resetBtn.addEventListener('click', () => { for (const square of squares) { square.className = ''; } displayCurrentPlayer.textContent = 1; }); const getPlayerOnSquare = square => { if (!square) return null; const { classList } = square; return classList.contains('player-1') ? 1 : classList.contains('player-2') ? 2 : null; }; function checkBoardForWinner() { for (let baseSquareIndex = 0; baseSquareIndex < squares.length; baseSquareIndex++) { const directionIndexDifferences = [ -6, // Northeast 1, // East 8, // Southeast 7 // South ]; // Look 4 squares in each direction from baseSquareIndex // If each of those squares has the same player, they win const playersOnSquares = directionIndexDifferences.map( difference => Array.from( { length: 4 }, (_, i) => getPlayerOnSquare(squares[baseSquareIndex + (i * difference)]) ) ); const foundSequence = playersOnSquares.find( players => players[0] && players.every(player => player === players[0]) ); if (foundSequence) { result.textContent = `Player ${foundSequence[0]} wins!`; } } }

* { box-sizing: border-box; text-align: center; } @font-face { font-family: BlackChancery; src: url(../blkChancery.ttf) format('truetype'); } body { background: teal; font-family: BlackChancery; font-size: 1.17rem; /* Nice golden yellow color 252, 186, 3 */ } .grid { margin: auto; margin-top: calc(2vh + 120px); border: solid 1px black; overflow: hidden; transform: scale(2.4); /* translate(-50%,-50%) */ display: flex; align-content: center; justify-content: center; flex-wrap: wrap; background-image: linear-gradient(-33deg, #384e65, #2e2e2e); height: 124px; width: 144px; } .grid div { height: 20px; width: 20px; border-radius: 50%; background: white; border: solid 1.8px black; filter: hue-rotate(0deg) blur(0.44px); transition: all 980ms ease-in-out; } .player-1 { background-image: linear-gradient(-33deg, red, rgb(186, 4, 119), rgb(186, 4, 119))!important; border-radius: 10px; filter: hue-rotate(0deg) blur(0.34px); transform: rotate(0deg); transition: all 980ms ease-in-out; } .player-2 { background-image: linear-gradient(-33deg, blue, rgb(4, 186, 156), rgb(4, 186, 156))!important; border-radius: 10px; filter: hue-rotate(0deg) blur(0.34px); transform: rotate(0deg); transition: all 980ms ease-in-out; } .player-one:hover { filter: hue-rotate(-49deg) blur(0.34px); transform: rotate(54deg); transition: all 980ms ease-in-out; } .player-two:hover { filter: hue-rotate(47deg) blur(0.34px); transform: rotate(54deg); transition: all 980ms ease-in-out; } #current-player { position: relative; background-image: linear-gradient(-33deg, red, rgb(186, 4, 119), rgb(186, 4, 119))!important; filter: blur(0.588px); text-shadow: 1px 1px 1.7px white; border: solid 2.98px black; width: 148px !important; height: 48px !important; font-size: 1.94rem; border-radius: 50%; padding: 8px 28px; margin-top: 8px; top: 8px; } #Reset { margin-top: calc(1.8vh + 78px); padding: 4px 8px 4px 8px; border-radius: 7px; background: rgb(67, 240, 211); font-size: 1.24rem; } /* position:absolute; top:50vh; left:50vw; */

<h1>Custom Connect Four<br>2 player Game</h1> <h3>The Current Player<br>is: Player <br><span id="current-player" width="50">1</span></h3> <h3 id="result"></h3> <div class="grid"> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> <div></div> </div><br> <button id="Reset">Reset</button> <script src="https://cdnjs.cloudflare.com/ajax/libs/animejs/3.2.0/anime.min.js" integrity="sha256-hBMojZuWKocCflyaG8T19KBq9OlTlK39CTxb8AUWKhY=" crossorigin="anonymous"></script>