for loop only applying to last item

Question

I'm working on a simple drag & drop activity that checks to see if all the drag items are in the correct drop zones. Things will get shuffled, but essentially the way I was looking at doing it was each drop zone checks to see if it contains "drag" + the matching array # (ie. the first array item drag0 sits within the first array item [0] drop zone).

However it's only really working for the last item (you can just get the last item right, and you'll get a correct response). I've seen options for wrapping it in a function, or using throw/catch, but nothing seems to work.

item class is draggable items, dropzone is the class for droppable areas.

function seeResult() {
  var result = document.getElementById("result");
  var drags = document.getElementsByClassName("item");
  var drops = document.getElementsByClassName("dropzone");
  var i;
  for (i = 0; i < drops.length; i++) {
    var num = i;
    if (drops[i].innerHTML.indexOf("drag" + num)) {
      result.style.color = "green";
    } else {
      result.style.color = "red";
    }
  }
};

<div class="item" draggable="true" ondragstart="drag(event)" id="drag0">Drag Item 1</div>
<div class="item" draggable="true" ondragstart="drag(event)" id="drag1">Drag Item 2</div>
<div class="item" draggable="true" ondragstart="drag(event)" id="drag2">Drag Item 3</div>
<div class="item" draggable="true" ondragstart="drag(event)" id="drag3">Drag Item 4</div>

<div id="dropZones">
  <div id="dropZone1" class="dropcon">
    <div class="dropzone" ondrop="drop(event)" ondragover="allowDrop(event)"></div>
    <div class="dragdesc">
      <p>Drop Description 1</p>
    </div>
  </div>
  <div id="dropZone2" class="dropcon">
    <div class="dropzone" ondrop="drop(event)" ondragover="allowDrop(event)"></div>
    <div class="dragdesc">
      <p>Drop Description 2</p>
    </div>
  </div>
  <div id="dropZone3" class="dropcon">
    <div class="dropzone" ondrop="drop(event)" ondragover="allowDrop(event)"></div>
    <div class="dragdesc">
      <p>Drop Description 3</p>
    </div>
  </div>
  <div id="dropZone4" class="dropcon">
    <div class="dropzone" ondrop="drop(event)" ondragover="allowDrop(event)"></div>
    <div class="dragdesc">
      <p>Drop Description 4</p>
    </div>
  </div>
</div>

<div id="seeResult" onclick="seeResult()">See result!</div>
<div id="result">Result</div>

One last note is that this will be added into a site where content creators can add/remove however many drag/drop items they want, and then spit it out to the end user, so things have to be fairly dynamic, can't just use these 4 specific array items.

Thanks in advance for your help!

Possible duplicate of [JavaScript closure inside loops – simple practical example](https://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example) — ajm, Nov 29 '18 at 14:34

ControlAltDel · Accepted Answer · 2018-11-29T15:02:25.030

1

The way to do this is to start it off assuming everything's good, and then if you find something that's not good, change it. See how I've changed your code to set result.style.color initially to green, then if you find an element that's not right, it sets it to red and returns.

function seeResult() {
    var result = document.getElementById("result");
    result.style.color = "green";
    var drags = document.getElementsByClassName("item");
    var drops = document.getElementsByClassName("dropzone");
    var i;
    for (i = 0; i < drops.length; i++){
        var num = i;
            if(drops[i].innerHTML.indexOf("drag"+num) == -1){
              result.style.color = "red";
              return;                
            }
    }
}

edited Nov 29 '18 at 15:02

answered Nov 29 '18 at 14:36

ControlAltDel

33,923
10
53
80

Ah, I like the idea of correct and then look for wrong; however I think there's still a problem with either the for loop or the if statement, I'm getting correct if any of the drag items land in any of the drop zones. – Tyler B Nov 29 '18 at 14:58
@TylerB yes there was a problem in the if statement. I refactored the code to quickly. Anyway, I have corrected this now and it should work – ControlAltDel Nov 29 '18 at 15:03
Ah yes I see, the == -1. Makes sense. What is the return doing exactly? (Sorry, I'm new, still learning). Thanks so much for taking the time to help, I really appreciate it! – Tyler B Nov 29 '18 at 15:20
Return is short-circuiting the for loop. It is unnecessary to keep checking all of the `drops` after one has already "failed" = entered the `if` statement because it doesn't have a "drag" in it. It only takes one failure to turn result red. – ControlAltDel Nov 29 '18 at 15:24
Oh great! Thanks again for all your help! – Tyler B Nov 29 '18 at 15:36

for loop only applying to last item

1 Answers1