javascript argument scoping issue and function return

Question

I have 2 slight issues with scoping in my function:

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...
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

re 2. ... it's when if (pointNotVisited) that the function clearly returns undefined - as there is no return statement in the "true" block — Jaromanda X
– Jaromanda X, Commented Sep 9, 2017 at 12:13
maybe i'll give some context here. It's a function which searches for a group of connected stones on a Go (chinese board game) board. I return inside because it only makes sense (to me at least) to recursively call the function on each of the neighbouring elements on the board. It is quite possible though, that I got lost somewhere inside my logic... — Jerzyn
– Jerzyn, Commented Sep 9, 2017 at 12:19
my point is, doing a return in forEach is redundant, as nothing comes of the return value — Jaromanda X
– Jaromanda X, Commented Sep 9, 2017 at 12:20
makes sense. however I'm not sure how would I call getGroup on the neighbours? — Jerzyn
– Jerzyn, Commented Sep 9, 2017 at 12:21

skirtle · Accepted Answer · 2017-09-09 17:56:35Z

1

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.

answered Sep 9, 2017 at 17:56

skirtle

29.3k4 gold badges49 silver badges63 bronze badges

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

3 Comments

Jerzyn Over a year ago

unfortunately this solution leads to 'call stack exceeded' error. I can't see why though :/

skirtle Over a year ago

Difficult to debug without the code. I suggest you create a JSFiddle.

skirtle Over a year ago

The inner function needs to call itself recursively, not call getGroup, line 104.

Collectives™ on Stack Overflow

javascript argument scoping issue and function return

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related