Check if user selects correct Spanish word article

Question 1

All Spanish nouns have a gender (masculine [el] or feminine [la]; e.g. la casa, el perro). I’m using some javascript and jquery to display a random word from a list of nouns (2D array), as well as buttons corresponding to the two Spanish articles (“el” or “la”). If the user clicks on the correct article for the word, a new one loads; else, the user sees an alert and is prompted to choose the correct article before loading a new word. I made this work on codepen, but I wasn’t able to get it to work correctly without reloading the page. All feedback will be well received.

$(document).ready(function() {
  setWord();

  function setWord() {

    //returns either 0 or 1
    var elOrLa = Math.round(Math.random());

    //wordList[0] contains 'la' nouns; wordList[1] contains those 'el' nouns
    var wordList = [["casa", "comida", "chica"],["perro", "amigo", "muchacho"]];

    // Return random int between min (included) and max (excluded)
    function getRandomInt(min, max) {
      min = Math.ceil(min);
      max = Math.floor(max);
      return Math.floor(Math.random() * (max - min)) + min;
    }

    //maximum length of each subarray
    var laLength = wordList[0].length;
    var elLength = wordList[1].length;

    //random index for subarrays
    var randomLaIndex = getRandomInt(0, laLength);
    var randomElIndex = getRandomInt(0, elLength);

    var article1 = "la ";
    var article2 = "el ";
    var word;
    var articlePlusWord;

    //sets random word and articlePlusWord
    if (elOrLa === 0) {
      word = wordList[0][randomLaIndex];
      articlePlusWord = article1.concat(word);
    } else {
      word = wordList[1][randomElIndex];
      articlePlusWord = article2.concat(word);
    }

    $("#test-word").text(word);

    if (elOrLa === 0) {
      $("#la-btn").on("click", function() {
        location.reload(true);
      });
      $("#el-btn").on("click", function() {
        alert("TOMA NOTA: ".concat(articlePlusWord));
        $("#el-btn").fadeOut();       
        $("#la-btn").css({"background":"limegreen"});
        $("#test-word").addClass('bg-danger').removeClass('bg-warning');
      });
    } else {
      $("#el-btn").on("click", function() {
        location.reload(true);
      });
      $("#la-btn").on("click", function() {
        alert("TOMA NOTA: ".concat(articlePlusWord));
        $("#la-btn").fadeOut();      
        $("#el-btn").css({"background":"limegreen"});
        $("#test-word").addClass('bg-danger').removeClass('bg-warning');
      });
    }
  };
});

<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.1.0/jquery.min.js"></script>
<h1 class="text-center"><span class="bg-warning" id="test-word"></span></h1>
<br>
<div class="container">
  <button type="button" class="btn btn-primary btn-lg" id="el-btn">EL</button>
  <button type="button" class="btn btn-primary pull-right btn-lg" id="la-btn">LA</button>
</div>

Expand snippet

Question 2

Message for moderators

As the OP already noticed, there is a strange issue when executing this in the context of the SO snippet:

uncaught exception: unknown (can’t convert to string)

I experienced the same with my totally refactored version, while it works without error in the usual context of my browser.

I can’t see a good way to inquire about that, so TIA if one of the SO developers could look at it and explain it!

Now the review

This is properly written, and I see no big flaws.
But the code might be significantly reduced, by merely avoiding useless parts and repetition.

Useless parts consist of preparing data for both “la” and “el” cases, while each execution will only use one of those two articles.
So I suggest to prepare data for the involved one only, as soon as it was chosen.

Repetition appears in the click events handling, where you precisely define what must happen for each button, and in both cases depending it’s the right one or not.
There too, we can compact this by acting for the two buttons at once, and evaluating if it’s right during the flow.

Here is a working snippet based on yours, where I included these suggestions:

function setWord() {
  
    // Return random int between min (included) and max (excluded)
    function getRandomInt(min, max) {
        min = Math.ceil(min);
        max = Math.floor(max);
        return Math.floor(Math.random() * (max - min)) + min;
    }
    
    const words = [
        ["casa", "comida", "chica"],
        ["perro", "amigo", "muchacho"]
    ];
    const articles = ['la', 'el'];
    
    var $buttons = $('.btn');
    var elOrLa = Math.round(Math.random());
    var article = articles[elOrLa];
    var $goodBtn = $('#' + article + '-btn');
    
    var $word = $('#test-word');
    var word = words[elOrLa][getRandomInt(0, words[elOrLa].length)];
    $word.text(word);
    
    $buttons.click(function() {
        if (this.id == $goodBtn[0].id) {
            location.reload(true);
        } else {
            alert('TOMA NOTA: ' + article + ' ' + word);
            $buttons.not($goodBtn).fadeOut();
            $goodBtn.css({'background': 'limegreen'});
            $word.addClass('bg-danger').removeClass('bg-warning');
        }
    });
}

setWord();

<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<h1 class="text-center"><span class="bg-warning" id="test-word"></span></h1>
<br>
<div class="container">
  <button type="button" class="btn btn-primary btn-lg" id="el-btn">EL</button>
  <button type="button" class="btn btn-primary pull-right btn-lg" id="la-btn">LA</button>
</div>

Expand snippet

Apart from the changes exposed above, you can see that:

I dropped the $(document).ready() wrapper: in most cases (and sure here) it’s not needed, as soon as the script appears after the HTML description part
I also moved the getRandomInt() function, which is now the 1st thing encountered in the setWord() function: this is a good habit to take now, since it becomes mandatory if you’re using "use strict"; (see this page)

Leave a Reply Cancel reply