CODEWITHSUNDEEP
javascriptnested-loops
Alpha Centauri A B
Alpha Centauri A B

Reputation: 143

Having issues with nested loops

When running the moveRight() function I am getting an error with it not recognizing map[x][y], saying that is is undefined. This only happens when "Player" is in the last y loop. I don't understand why this is happening, could somebody help explain?

var map = [
    ["Blank", "Blank", "Blank", "Blank"],
    ["Blank", "Blank", "Blank", "Blank"],
    ["Blank", "Blank", "Blank", "Blank"],
    ["Blank", "Player", "Blank", "Blank"],
    ["Blank", "Blank", "Blank", "Blank"],
    ["Blank", "Blank", "Blank", "Blank"],
    ["Blank", "Blank", "Blank", "Blank"]
];

function moveRight() {
    var breakLoop = false;
    for (y = 0; y < map.length; y++) {
        for (x = 0; x < map[y].length; x++) {

            var posX = map[x][y].indexOf("Player");
            if (posX <= -1) continue;
            if (y >= map[y].length-1) {
                breakLoop = true;
                break;
            }

            breakLoop = true;
            console.log("x: " + x);
            console.log("y: " + y);

            map[x][y] = "Blank";
            map[x][y+1] = "Player"; 
            break;
        }
        if (breakLoop) break;
    }
}

Upvotes: 2

Views: 144

Answers (2)

John Kugelman
John Kugelman

Reputation: 361675

The way you've written your loops with y in the outer loop and x in the inner one, the map needs to be accessed by y first, then x.

var posX = map[y].indexOf("Player");

Then the y bounds check should instead check x, which makes sense since this is a horizontal movement.

if (x >= map[y].length - 1) {

The movement lines should be:

map[y][x] = "Blank";
map[y][x+1] = "Player";

Additionally, it's a good idea to add var to your local variable declarations so they don't leak into the global scope.

for (var y = 0; y < map.length; y++) {
    for (var x = 0; x < map[y].length; x++) {

Finally, it doesn't look like you need the inner loop. You're searching through each row with indexOf so there's no need to iterate over each individual square in the rows. Which means posX can become x.

Applying all of these ideas, here's the final code. Notice how some careful refactoring lets us get rid of the breakLoop variable as well.

for (var y = 0; y < map.length; y++) {
    var x = map[y].indexOf("Player");

    if (x <= -1) {
        continue;
    }

    if (x < map[y].length - 1) {
        console.log("x: " + x);
        console.log("y: " + y);

        map[y][x]     = "Blank";
        map[y][x + 1] = "Player"; 
    }

    break;
}

Upvotes: 2

JaredPar
JaredPar

Reputation: 754763

The loops are structured to have y be the first index and x be the second. In the following 2 lines though you are using them in the wrong order

map[x][y] = "Blank";
map[x][y+1] = "Player";

It should be

map[y][x] = "Blank";
map[y + 1][x] = "Player";

Additionally the y + 1 index is suspicious. On the last iteration of the loop it will be outside the bounds of the array. Did you mean to change the loop conditional to be y + 1 < map.length?

Upvotes: 1

Related Questions