1

I've created a nodeList of list elements using createElement(). Then I've used Array.from() to convert the nodeList in question into array I can iterate over. I want to apply a different width according to the value of the index. If index is even width of 300px else width of 500px. However, the console returns "Cannot read property 'style' of undefined at bottlesOnTheWall".

I've also used [...] to turn my nodeList into an array but without success either. My guess is that is not a nodeList in the first place, which means it can't be converted into an array. At least not using either of these approaches.

I was wondering if someone could point out where I've gone wrong and fix my code. I've been spending more time that's healthy trying to do so.

const bottles = document.getElementById("bottles");
count = 99;
var myArray = [];
var j = 0;

function bottlesOnTheWall() {
  while (count > 0) {
    if(count > 2) {
      myArray.push(`${count} bottles of beer on the wall, ${count} bottles of beers. Take one down and pass it around, ${count - 1} bottles of beer on the wall`)
    } else if (count === 2) {
      myArray.push(`${count} bottles of beer on the wall, ${count} bottles of beers. Take one down and pass it around, ${count - 1}bottle of beer on the wall`)
    } else if (count === 1) {
      myArray.push(`${count} bottle of beer on the wall, ${count} bottles of beers. No more bottle of beer on the wall`)
    } else {
      myArray.push(`No more bottles of beer on the wall. No more bottles of beer.  Go to the shop and buy some ${count} more.`)
    }
    count--
  }
  while (j < myArray.length) {
    var liElement = document.createElement("li");
    var text = document.createTextNode(myArray[j]);
    liElement.appendChild(text);
    bottles.appendChild(liElement);
    var bottlesArray = Array.from(bottles);
    if(j % 2) {
      bottlesArray[j].style.width = "300px";
    } else {
      bottlesArray[j].style.width = "500px";
    }
    j++;
  }
}
bottlesOnTheWall();
#bottles {
  line-height: 2;
  letter-spacing: 3px;
}

/* ul {
  list-style-image: url('beer.png');
} */

body {
  background: #FFF8DC;
}

li {
  max-width: 500px;
  margin: auto;
  margin-bottom: 10px;
}

ul li {
  background: #FFEBCD;
}
<!DOCTYPE html>
<html lang="en" dir="ltr">
  <head>
    <meta charset="utf-8">
    <title>bottles on the wall</title>
    <link rel="stylesheet" href="index.css">
  </head>
  <body>
    <ul id="bottles" ></ul>
    <script src="index.js" type="text/javascript"></script>

  </body>
</html>

3
  • 1
    const bottles = document.getElementById("bottles"); returns a node, not a nodelist. Array.from(bottles) therefore tries to convert a node to an array. Commented May 2, 2021 at 8:32
  • Also, I'm not really sure why you're converting to an array bottlesArray[j].style.width = "some width"; means you're converting to an array just to get the last index j out of it. It can probably just be liElement.style.width = "some width"; if you want the last appended element. Commented May 2, 2021 at 8:34
  • another question is, why do you create an array first and then iterates it? Commented May 2, 2021 at 8:39

2 Answers 2

2

Array.from needs a variable with an implemented Symbol.iterator. A single element does not have it, but a list of elements, which is not given here.

Then you have some more issues:

  • Global variables, but only used in a single function. Just move all varaibles inside of the function.

  • Take a parameter for count.

  • Take a single loop without collecting all texts first in an array and then iterate again for creating elements. The only purpose is to use a layer model to separate the data collection from the presentation layer.

  • Finally take a conditional (ternary) operator ?: for width.

function bottlesOnTheWall(count) {
    const bottles = document.getElementById("bottles");
    
    while (count > 0) {
        let text;

        if (count > 2) text = `${count} bottles of beer on the wall, ${count} bottles of beers. Take one down and pass it around, ${count - 1} bottles of beer on the wall`;
        else if (count === 2) text = `${count} bottles of beer on the wall, ${count} bottles of beers. Take one down and pass it around, ${count - 1} bottle of beer on the wall`;
        else if (count === 1) text = `${count} bottle of beer on the wall, ${count} bottles of beers. No more bottle of beer on the wall`;
        else text = `No more bottles of beer on the wall. No more bottles of beer.  Go to the shop and buy some ${count} more.`;

        count--;

        const liElement = document.createElement("li");
        liElement.appendChild(document.createTextNode(text));
        bottles.appendChild(liElement);
        liElement.style.width = count % 2
            ? "300px"
            : "500px";
    }
}

bottlesOnTheWall(99);
#bottles { line-height: 2; letter-spacing: 3px; }
body { background: #FFF8DC; }
li { max-width: 500px; margin: auto; margin-bottom: 10px; }
ul li { background: #FFEBCD; }
<ul id="bottles"></ul>

Sign up to request clarification or add additional context in comments.

Comments

2

You can drive the DRY principle even a bit further and shorten the script by doing so. The alternating widths of the <li>s can be enforced by using a simple CSS selector: li:nth-child(even) { max-width:300px }.

The function bob(n) generates a string specifying a number of bottles of beer and is defined within the bottlesOnTheWall() function. It can then be used in any place and will supply a properly inflected string with any number of n.

function bottlesOnTheWall(count) {
  let n=count;
  const LIs=[],
        bob=n=>(n>1?n:(n==0?'no more':1))+' bottle'+(n==1?'':'s')+' of beer';       
  do {
    LIs.push('<li>'+bob(n)[0].toUpperCase()+bob(n).substring(1)+' on the wall, '
    +bob(n)+'. '
    +(n?`Take one down and pass it around, ${bob(n-1)} on the wall`
       :`No more bottles of beer.  Go to the shop and buy some ${count} more.`)
    +'</li>'
    );
  } while(n--)
  document.getElementById("bottles").innerHTML=LIs.join('\n')    
}

bottlesOnTheWall(9);
#bottles { line-height: 2; letter-spacing: 3px; }
body { background: #FFF8DC; }
li { max-width:500px; margin: auto; margin-bottom: 10px; }
li:nth-child(even) { max-width:300px }
ul li { background: #FFEBCD; }
<ul id="bottles"></ul>

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.