javascript argument scoping issue and function return

61 views Asked by At

I have 2 slight issues with scoping in my function:

  1. First is with scoping of the liberties argument. Everything goes fine until it reaches the main 'else' block when the variable gets reset when returning to a previous recursion. Putting liberties as a global variable fixes the problem, but that's not how it should be done...

  2. the last return in the function gives undefined. Even though I can easily console.log those variables (they are correct in all cases, besides liberties if not global). I don't see any asynchronous behaviour here and it puzzles me how does it work.

function getGroup(position, board, visited = [], group = [], liberties = 0) {
    let pointNotVisited = visited.reduce((acc, cur) => (cur.x !== position.x || cur.y !== position.y) && acc, true)
    
    if (pointNotVisited) {
        let neighbours = getNeighbours(position, board)

        group.push(position)
        visited.push(position)

        let unvisitedNeighbours = neighbours.reduce((acc, cur) => {
            let neighbourVisited = visited.filter(el => (el.x === cur.x && el.y === cur.y))
            if (neighbourVisited.length > 0) {
                return acc
            } else {
                return acc.concat(cur)
            }
        }, [])
        
        unvisitedNeighbours.forEach(stone => {
            if (stone.color == 'empty') {
                liberties++
                visited.push(stone)
            } else if (stone.color != position.color) {
                visited.push(stone)
            }
            return getGroup(stone, board, visited, group)
        })
    } else {
        return {
            'group': group,
            'liberties': liberties
        }
    }
}

NOTE: position has a format of: { x: 1, y: 3, color: "white" }, board is used for the neighbours function. Let me know if you need any other info :) Please advice. Thanks in advance!

EDIT: Thanks to Jaromanda X I fixed the return value issue by:

unvisitedNeighbours.forEach(stone => {
    if (stone.color == 'empty') {
        liberties++
        visited.push(stone)
            // console.log('liberties after increment', liberties)
    } else if (stone.color != position.color) {
        visited.push(stone)
    }
    // console.log('liberties before recursion', liberties)
    getGroup(stone, board, visited, group, liberties)
})
return {
    "group": group,
    "liberties": liberties
}

however the liberties scoping issue is still there

1

There are 1 answers

3
skirtle On BEST ANSWER

You can capture the liberties variable in a closure using an inner function rather than trying to pass it around. It would look something like this:

function getGroup(position, board, visited = [], group = []) {
    var liberties = 0

    return myInnerFn(position, board, visited, group)

    function myInnerFn(position, board, visited, group) {
       // ... does grouping logic, recursively calling itself ...
    }
}

Here myInnerFn should perform all the logic you have in your getGroups, only liberties has been moved out. You may want to consider moving out other variables too, I'm not sure you need to pass around board, visited or group once you have a closure. I'm also guessing that visited and group will no longer be needed as arguments of the outer function, they can be created as variables just like liberties.

By using two functions we introduce an extra level to our variable scoping, allowing us to push the variable up out of the inner function without having to go all the way to global.