CODEWITHSUNDEEP
javascriptfunctionloops
Alejandro Arias Aguilar
Alejandro Arias Aguilar

Reputation: 63

Javascript - Functions won´t run the same way after the first time

I am new learning front-end and Javascript and I am designing a game ¨to learn the abecedary" on my own to achieve that.

My intention is to:

Now, I have achieved all of these steps. But when I keep playing and select the letter C (or any other letter), the code won´t split between the 2nd and 3rd steps, and it assigns both classes (firstSelectedLetter and secondSelectedLetter) to the clicked letter. It compares the same letter and of course it´s wrong and the game ends.

See the console logs here:

The first time it has found out that the letter A has a 1, and the letter B has a 2. It compares both of them and says that I am right because the 2 = 1+1.

Buuut, when I keep playing and I choose letter C it just assigns both classes, compares the same latter and booom.

See the log fromt the DevTools

I have tried a lot of combinations, and tons of work arounds. But I am pretty sure I am missing something that I have not yet learned. I hope you guys can teach that to me and help others :)

Here´s the fiddle for your reference: https://jsfiddle.net/alejandroarag/qb91kean/

    // First I create the array with the abecedary letters
var abecedary = ["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"];


// Second, I use this function to shuffle the letters randomly
function shuffleAbecedary() {
    var result;
    result = abecedary.sort(function () {
        return 0.5 - Math.random();
    });
    return result;
}

//From this function i got on the internet i get the position on the letters on the alphabet (like 1st, 2nd...)
function alphabetPosition(text) {
    var result = "";
    for (var i = 0; i < text.length; i++) {
        var code = text.toUpperCase().charCodeAt(i)
        if (code > 64 && code < 91) result += (code - 64) + " ";
    }

    return result.slice(0, result.length - 1);
}

// This function prints the letter in a DIV with a class and a dataset value that I will use later on
function lettersPrint() {
    var gameBoard = document.querySelector("#gameboard");
    var shuffledLetters = shuffleAbecedary();
    gameBoard.innerHTML = "";

    shuffledLetters.forEach(function (element) {
        var letter = document.createElement("div");
        letter.innerHTML = "<div class='letter' data-valor= " + element + ">" +
            "<div class='letter_content'>" +
            element +
            "</div>" +
            "</div>";
        //I put all of them inside the gameBoard ID
        gameBoard.appendChild(letter);
    });

}

function firstToSelect() {
    var firstSelected = document.querySelectorAll(".firstLetterSelected:not(.correct)");

    if (firstSelected.length > 0) {
        return
    }

    this.classList.add("selected");
    this.classList.add("firstLetterSelected");

    document.querySelectorAll(".letter:not(.selected)").forEach(function (element) {
        element.addEventListener("click", secondToSelect);
    });
}


function secondToSelect() {
    var firstSelected = document.querySelectorAll(".firstLetterSelected:not(.correct)");
    var secondSelected = document.querySelectorAll(".secondLetterSelected:not(.correct)");

    if (secondSelected.length > 0) {
        return
    }

    this.classList.add("selected");
    this.classList.add("secondLetterSelected");

    var lettersToCompare = firstSelected + secondSelected;


    compare(lettersToCompare);
}

function compare(lettersToCompare) {
    var firstLetter = document.querySelectorAll(".firstLetterSelected:not(.correct)");
    var secondLetter = document.querySelectorAll(".secondLetterSelected:not(.correct)");
    var firstLetterToCompare = alphabetPosition(firstLetter[0].dataset.valor);
    var secondLetterToCompare = alphabetPosition(secondLetter[0].dataset.valor);

    var lettersToCompare = firstLetterToCompare + "," + secondLetterToCompare;

    var theLetters = document.querySelectorAll(".selected:not(.correct)")
    var letsCompare = lettersToCompare.split(",");

    console.log(letsCompare[0]);
    console.log(letsCompare[1]);

    if (letsCompare[0] == letsCompare[1] - 1) {
        console.log("You are right!");
        correct(theLetters);

    } else {
        console.log("Opps you are not right!");
        error(theLetters);
    }
}

function correct(theLetters) {
    theLetters.forEach(function (element) {
        element.classList.add("correct");
    });
}

function error(theLetters) {


    //    theLetters.forEach(function (element) {
    //        element.classList.add("error");
    //
    //        setTimeout(
    //            function () {
    //                theLetters.forEach(function (element) {
    //                    element.classList.remove("selected");
    //                    element.classList.remove("error");
    //                });
    //            }, 1000);
    //    });
}

lettersPrint();
document.querySelectorAll(".letter").forEach(function (element) {
    element.addEventListener("click", firstToSelect);
});

Upvotes: 1

Views: 116

Answers (2)

Tigger
Tigger

Reputation: 9120

Your code appears to be overly complex from what I am understanding you are wanting to achieve. This makes me think I may have misunderstood the objective and game logic. If that is case, the following simplification may not be appropriate.

Basically, I think you can cut back on most of the code.

// first letter should always be A? If so, we can set lastSelected = (charCode 64). 
var lastSelected = String.fromCharCode(64), abecedary = ["A", "B", "C", "D", "E", "F", "G", "H", "I", "J", "K", "L", "M", "N", "O", "P", "Q", "R", "S", "T", "U", "V", "W", "X", "Y", "Z"];

// shuffle the letters randomly
function shuffle(arr) {
    var result = arr.sort(function () {
        return 0.5 - Math.random();
    });
    return result;
}

function checkOrder(e){
  var s = this.textContent;
  if ((s.charCodeAt(0) - 1) != lastSelected.charCodeAt(0)){
    this.classList.add("err");
  } else {
    this.classList.remove("err");
    this.classList.add("correct");
    lastSelected = s;
  }
}

shuffle(abecedary);

var l, board = document.getElementById("board");
for(i=0;i<abecedary.length;i++){
  l = document.createElement("div");
  l.setAttribute("class","letter");
  l.textContent = abecedary[i];
  l.addEventListener("click",checkOrder,false);
  board.appendChild(l);
}
#board {
  max-width:216px;
}
.letter {
  display:inline-block;
  width:24px;
  line-height:24px;
  text-align:center;
}
.err {
  background:#f99;
}
.correct {
  background:#9f9;
}
<div id="board"></div>

Upvotes: 1

edoput
edoput

Reputation: 1212

I've taken a look and I think the big problem is here

    document.querySelectorAll(".letter:not(.selected)").forEach(function (element) {
        element.addEventListener("click", secondToSelect);
    });

You register a second click listener but never get rid of it?

First of all I like the game, it's a nice exercise for you.

You definitely need to do move some of the hidden state of the game out of the HTML and CSS and into the code, you will be able to achieve much more with a little bit of shuffling responsibility around.

Right now the state is in the CSS classes and you fetch which one is the first and which one is the second by looking at it but you are making it complicated; here's what I would do.


var choices = [] // a list of letters chosen by the player

function isCorrect() {
    return choices.length == 2 & choices[0] < choices[1] // this is just for two letters at a time but could be made longer
}

function clear() {
  choices = []
}

function select(a) {
  if (choices.length == 2) {
    return
  }
  choices.push(a)
}

Then instead of registering 2 click listener I would register 1 that would use select to add the value to the choices and then call isCorrect to know if it should update the UI and state (calling clear and removing letters from the board) or not.

Bonus: you can compare a lot of values in javascript and in this case you should try "a" < "b" and see what happens!

Upvotes: 1

Related Questions