Make method less repetative

I may be reading your code wrong but, it seems kind of redundant to repeat the

return { row, column in

let newCol = -2 * column

Also, it seems to me you only need the

default:

return (newCol, 0)

once for instance in the final else clause.

Could you explain how? If I move the "return" part to the very top, for example, then the method is doing all the if checks, at which point there's no benefit to returning a function.

Again, I may be reading your code incorrect but something like:

private func generateLoserLocationFunction(numberOfPlayers: Int) -> (column: Int, row: Int) -> (column: Int, row: Int) {

return { row, column in

let newCol = -2 * column

switch numberOfPlayers {

case 8:

switch column {

case 0:

return (newCol, row / 2)

case 1:

return (newCol, row ^ 1)

}

case 16:

switch column {

case 0:

return (newCol, row / 2)

case 1:

return (newCol, 3 - row)

case 2:

return (newCol, row ^ 1)

}

case 32:

switch column {

case 0:

return (newCol, row / 2)

case 1:

return (newCol, (row + 4) % 8)

case 2:

return (newCol, row)

case 3:

return (newCol, row ^ 1)

}

default:

return (newCol,0)

}

}

It's not a big change, 35 lines instead of 50 but if I'm reading it right, it should do the same thing your existing version

does.

Although not understand the purpose of your code (which makes it difficult to propose any improvement), try this:

private func generateLoserLocationFunction(numberOfPlayers: Int) -> (column: Int, row: Int) -> (column: Int, row: Int) {
    switch numberOfPlayers {
    case 8,16,32:
        return { row, column in
            let newCol = -2 * column
            switch column {
            case 0:
                return (newCol, row / 2)
            case 1 where numberOfPlayers == 8:
                return (newCol, row ^ 1)
            case 1 where numberOfPlayers == 16:
                return (newCol, 3 - row)
            case 1 where numberOfPlayers == 32:
                return (newCol, (row + 4) % 8)
            case 2 where numberOfPlayers == 16:
                return (newCol, row ^ 1)
            case 2 where numberOfPlayers == 32:
                return (newCol, row)
            case 3 where numberOfPlayers == 32:
                return (newCol, row ^ 1)
            default:
                return (newCol, 0)
            }
        }
    default:abort()
    }
}

You also can "combo" some cases like this:

case 1,2 where numberOfPlayers < 32:
       return (newCol, row ^ 1)

But only you can tell whats make sense for your code.

The original version abort if numberOfPlayers is different from 8,16 or 32.... But just him should know if this is necessary.

Much cleaner 🙂 Hadn't thought of doing it that way 😉

Agreed, I assumed, perhaps wrongly, that the default return would be 0 in case evaluation

failed overall.

The intent is to return a method which will *not* have to check numberOfPlayers every time. That will never change after initial setup, so I don't want to have to do the check against it every single time I call this method. that's why originally I was doing three distinct return statements, as each returned just the code for that number of players.

Would it not do what you wish as Wallace suggested? Surely, number of players is the deciding factor...And being in

a switch/case pairing, only those statements that are true would be executed. Thinking on that, if you only need this

at initialization, set a flag and never call this method if set.

What wallace is showing is going to compare against numberOfPlayers every time the method is called. I'm trying to make the returned method NOT check against numberOfPlayers ever, as it only has to be checked the one time this outer method is called.

Maybe it's not clear the intent is something like:

let theMethodToUse = generateLoserLocationFunction(16)
let location = theMethodToUse(...)
let otherLocation = theMethodToUse(...)

So the "generateLoserLocationFunction" will only ever get called a single time. Then, the function it returns is what is called over and over. So in "theMethodToUse" I don't want it looking at the number of players.

Then my last thought is your answer. Set a flag. Test the flag before calling this method.

I don't understand what you are saying. I know 100% that generateLoserLocationFunction() will only ever be called a single time.

What you don't seem to be following now is that the method return, which I execute multiple times, will *always* include checks for numberOfPlayers, as it's part of the method body that was returned.

So you are deliberately rewriting Return. Don't do that! By subclassing Return, you are causing your

own issue.

I'm sorry, maybe I'm being totally dense, but what Wallace wrote would be no different than if I had just done this and not returned a new function:

func theMethodToCallOverAndOver(column: Int, row: Int) -> (column: Int, row: Int) {
    let newCol = -2 * column
    switch column {
    case 0:
        return (newCol, row / 2)
    case 1 where numberOfPlayers == 8:
        return (newCol, row ^ 1)
    case 1 where numberOfPlayers == 16:
        return (newCol, 3 - row)
    case 1 where numberOfPlayers == 32:
        return (newCol, (row + 4) % 8)
    case 2 where numberOfPlayers == 16:
        return (newCol, row ^ 1)
    case 2 where numberOfPlayers == 32:
        return (newCol, row)
    case 3:
        return (newCol, row ^ 1)
    default:
        return (newCol, 0)
    }
}

That means every time I call the method, I'm checking the value of numberOfPlayers.