Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript DOM Scripting By Example Editing and Filtering Names States of the Application

Posted by paulscanlon
paulscanlon
Courses Plus Student 26,735 Points

Bad Practice?

Hi guys.

At around 4:30. Guil uses this code.

ul.addEventListener("click", (e) => {
  if (e.target.tagName === "BUTTON") {
    if(e.target.textContent === "Remove") {
      const li = e.target.parentNode;
      const ul = li.parentNode;
     ul.removeChild(li);
     }
  }

Better or worse to do it like this?

ul.addEventListener("click", (e) => {
  if (e.target.tagName === "BUTTON" && e.target.textContent === "Remove") {

      const li = e.target.parentNode;
      const ul = li.parentNode;
      ul.removeChild(li);

  }

Thanks in advance

Paul

4 Answers

Joe Beltramo
Joe Beltramo
Courses Plus Student 22,191 Points

In the event that the nested if (the one within the outer if) was the only conditional statement, yes you would be correct. However in the video, Guil moves on to adding an else if within the statement which validates that having an if/else-if within an if statement is perfectly fine

To demonstrate:

// Bad

if (x === y) {
    if (a === b) {
        // do stuff
    }
}

// Good

if (x === y && a === b) {
    // do stuff
}

// However this is also good

if (x === y) {
    if (a === b) {
        // do stuff
    } else if (b === c) {
        // do some other stuff
    }
}


// While this is repetitive

if (x === y && a === b) {
    // do stuff
} else if (x === y && b === c) {
    // do some other stuff
}

The if/else-if method makes sure that no matter whether the internal if or else if are true, the outer if must have been true for the if/else-if to even been considered

The reason Guil took the approach he did, is because he intended to add an else-if

paulscanlon
paulscanlon
Courses Plus Student 26,735 Points

I see. Thanks for the detailed answer :)

Alexander Melo
Alexander Melo
4,297 Points
Alexander Melo
Alexander Melo
4,297 Points

Its not really bad practice because You have one if statement encapsulating if it is a button. What if you had 10 buttons? You can have ten additional if inside the button filter. But if you had ten buttons why repeat the && operators if you are still checking if its a button when one if statement can check it out for all 10?

Brian Burkard
Brian Burkard
5,771 Points
Brian Burkard
Brian Burkard
5,771 Points

If you don't do them nested with if else like this, sometimes the button will trigger too fast. I had it like you have it at first. And when I click on 'edit', it was simultaneously triggering my event for when my button is in the 'save' state. Even though I had it change at the end of my edit code block it still triggered too fast.

mourad marzouk
mourad marzouk
5,560 Points
mourad marzouk
mourad marzouk
5,560 Points

I mean I don't think you need the button section at all. This works for me: if (button.textContent === 'remove') { ul.removeChild(li); } else if (button.textContent === 'edit') { console.log('testing') }