Take words from input boxes and choose one randomly

Question

Im learning Javascript, HTML and CSS and for do some practice i decided to do a website that choose a word randomly from input boxes.

When I do the console.log the array is empty and obviously items.length does not works.

Tysm if u can help me and fix my code.

JS

function randomword() {
  var words = [document.getElementById("word1"), document.getElementById("word2"), document.getElementById("word3"), document.getElementById("word4")]
  console.log(words)
  var items = words[Math.floor(Math.random() * items.length)];
document.getElementById("wresult").innerHTML = items

HTML

<div id=wgen>
            <label for="name">Prima parola (obbligatoria):</label>
            <input type="text" id="word1" name="name" required
                minlength="1" maxlength="40" size="40">
            <label for="name">Seconda parola (obbligatoria):</label>
            <input type="text" id="word2" name="name" required
                    minlength="1" maxlength="40" size="40">                
            <label for="name">Terza parola</label>
            <input type="text" id="word3" name="name" required
                    minlength="1" maxlength="40" size="40">
            <label for="name">Quarta parola</label>
            <input type="text" id="word4" name="name" required
                     minlength="1" maxlength="40" size="40">  
            <button onclick="randomword()" id='randomword' type="button">Genera</button>
            <p id=wresult></p>                           
        </div>

You are populating your array with references to the `input` DOM elements, rather than the `value` of those elements. Make sure to add `.value` on the end of each reference. — Scott Marcus, Dec 29 '20 at 20:46
Also, do not use `.innerHTML` if you can avoid it as it has security and performance implications. Since the text you are working with does not contain any HTML that needs to be parsed, use `.textContent` instead. — Scott Marcus, Dec 29 '20 at 20:48
And, the `for` attribute is meant to have a value that matches the `id` of a particular form field, not the `name` attribute. And the `name` attribute is only helpful when you have a `form` that will be submitting it's field values somewhere, which doesn't seem to be the case here, so you should just remove the `name` attributes from the `input` elements. — Scott Marcus, Dec 29 '20 at 20:50

score 1 · Accepted Answer · answered Dec 29 '20 at 20:46

1

You want to get the value of the DOM elements with document.getElementById("word1").value.

Also, your array is not empty it will always have 4 elements in it, they'll just be the raw DOM nodes intead of the string value of their contents.

answered Dec 29 '20 at 20:46

Andy Ray

30,372
14
101
138

Your suggestion is useful, thx. I’m following a course on Udemy and I haven’t got to the DOM part yet and I tried. I can’t vote up it bc I don’t have enough “reputation”(?). So... I’ll work with ur code Next morning for do some experiments. Good night :) (here in Italy is 00:47) – Endxxr Dec 29 '20 at 23:47
When you determine which answer is most appropriate (it doesn't have to be mine), don't forget to click the empty check mark next to the question to help others find the correct answer in the future, and give the answer-ers fake internet points :) – Andy Ray Dec 29 '20 at 23:50

Spectric · Answer 2 · 2020-12-29T21:17:43.853

Try this:

var word1 = document.getElementById("word1");
var word2 = document.getElementById("word2");
var word3 = document.getElementById("word3");
var word4 = document.getElementById("word4");
function randomword() {
  var words = [word1.value, word2.value, word3.value, word4.value];
  console.log(words[random(0, words.length)]);
}

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

<div id=wgen>
  <label for="name">Prima parola (obbligatoria):</label>
  <input type="text" id="word1" name="name" required minlength="1" maxlength="40" size="40">
  <label for="name">Seconda parola (obbligatoria):</label>
  <input type="text" id="word2" name="name" required minlength="1" maxlength="40" size="40">
  <label for="name">Terza parola</label>
  <input type="text" id="word3" name="name" required minlength="1" maxlength="40" size="40">
  <label for="name">Quarta parola</label>
  <input type="text" id="word4" name="name" required minlength="1" maxlength="40" size="40">
  <button onclick="randomword()" id='randomword' type="button">Genera</button>
  <p id=wresult></p>
</div>

For better organization, we set each individual input to a specific variable, which we plug into words. Using our custom defined random method, we can generate a random index inside the array. We use this random index to get a random value.

To get the value of the input field, we use .value

It's not a good idea to do the DOM queries within the `randomword` function because you'll be querying over and over for the same elements. Get your DOM references just once. Also `.getElementById()` is faster than `.querySelector()` so use that when you have `id`s to work against. — Scott Marcus, Dec 29 '20 at 21:15

KooiInc · Answer 3 · 2020-12-29T21:50:43.847

It is generally not a good idea to use inline handlers. Furthermore you'll need the input values. Here's a snippet that may help you forward. It uses event delegation for the handling.

document.addEventListener("click", randomword);

function randomword(evt) {
  if (evt.target.id === "randomword") {
    //           ^ act only if button#randomword was clicked
    let words = [...document.querySelectorAll("input[type='text']")]
    //                                         ^ get the input elements
      .filter(input => input.id.startsWith("word") && input.value)
      // filter only all #wordx with value
      .map(input => input.value);
      // map to values only
    console.clear();

    // with less then 2 words randomizing doesn't make sense
    if (words.length >= 2) {
      let item = words[Math.floor(Math.random() * words.length)];
      console.log(`Random choice from given words: ${item}`);
    } else { 
      console.log(`no words available (yet)`);
    }
    return;
  }
  // action for clear button
  if (evt.target.id === "clear") {
      [...document.querySelectorAll("input[type='text']")]
       .forEach(input => input.id.startsWith("word") && (input.value = ""))

  }
}

<p>
  <label for="word1">Prima parola (obbligatoria):</label>
  <input type="text" id="word1" required minlength="1" maxlength="40" size="40" value="a">
</p>
<p>
  <label for="word2">Seconda parola (obbligatoria):</label>
  <input type="text" id="word2" required minlength="1" maxlength="40" size="40" value="b">
</p>
<p>
  <label for="word3">Terza parola</label>
  <input type="text" id="word3" required minlength="1" maxlength="40" size="40" value="c">
</p>
<p>
  <label for="word4">Quarta parola</label>
  <input type="text" id="word4" required minlength="1" maxlength="40" size="40" value="d">
  <button id='randomword' type="button">Genera</button>
  <button id='clear' type="button">Clear</button>
</p>

<p id=wresult></p>

[More complete list of reasons](https://stackoverflow.com/questions/43459890/javascript-function-doesnt-work-when-link-is-clicked/43459991#43459991) why inline event handlers should not be used. — Scott Marcus, Dec 29 '20 at 21:39

Take words from input boxes and choose one randomly

3 Answers3