Make method less repetative

If numberOfPlayers does not matter, why does this function exist? No matter how it is written you've

setup a looping condition. If it doesn't matter, rethink your path. You DON'T NEED a new function that

will be used only once. Recursively calling that function is useless. So, what is it, that you are actually

trying to accomplish?

Number of players matters to determine what the new function would look like. Think of it this way. I have to call a method over and over to get a color. The color will always be red for 8 players, blue for 16 players, and green for 32 players. So I could do this:

func getColor(numberOfPlayers: Int) -> UIColor {
    if numberOfPlayers == 8 {
        return UIColor.redColor()
    } else if numberOfPlayers == 16 {
        return UIColor.blueColor()
    } else if numberOfPlayers == 32 {
        return UIColor.greenColor()
    }
}

But that means when I ask for the color a million different times in various parts of the code, I have to do those if/else checks. So instead, I want to have a new getColor function defined that just returns the proper color. Since numberOfPlayers won't change, I can create the function to do the specific thing:

func generateGetColorFunction(numberOfPlayers: Int) -> () -> UIColor {
    if numberOfPlayers == 8 {
        return {
            return UIColor.redColor()
        }
    } else if numberOfPlayers == 16 {
        return {
            return UIColor.blueColor()
        }
    } else if numberOfPlayers == 32 {
        return {
            return UIColor.greenColor()
        }
    } else {
        abort()
    }
}
let getColor = generateGetColorFunction(32)
let iAmGreen = getColor()

So now, when I call getColor(), all it does is return the exact color with no specific checks. So if I send in 32 to generateGetColorFunction it's like I had just defined:

func getColor() -> UIColor {
   return UIColor.greenColor()
}

somewhere in my code.

This is the best I can come up with:

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

What about:

typealias ColRowMapper  =   (col:Int, row:Int) -> (col:Int, row:Int)
func gen(playerCount:Int) -> ColRowMapper {

    let case8:[Int:ColRowMapper] = [
        0:  {(r:Int,c:Int) in return (-2 * c, r / 2)}
    ,   1:  {(r:Int,c:Int) in return (-2 * c, r ^ 1)}
    ]

    let case16:[Int:ColRowMapper] = [
        0:  {(r:Int,c:Int) in return (-2 * c, r / 2)}
    ,   1:  {(r:Int,c:Int) in return (-2 * c, 3 - r)}
    ,   2:  {(r:Int,c:Int) in return (-2 * c, r ^ 1)}
    ]

    let case32:[Int:ColRowMapper] = [
        0:  {(r:Int,c:Int) in return (-2 * c, r / 2)}
        ,   1:  {(r:Int,c:Int) in return (-2 * c, (r + 4) % 8)}
        ,   2:  {(r:Int,c:Int) in return (-2 * c, r)}
        ,   3:  {(r:Int,c:Int) in return (-2 * c, r ^ 1)}
    ]

    let lookup:[Int:ColRowMapper] = [
        8:  { (r:Int,c:Int) in
            if let eval = case8[c] {
                return eval(col:c, row:r)
            }
            return (col:-2 * c, row:r)
        }

    ,   16: { (r:Int,c:Int) in
            if let eval = case16[c] {
                return eval(col:c, row:r)
            }
            return (col:-2 * c, row:0)
        }

    ,   32: { (r:Int,c:Int) in
            if let eval = case32[c] {
                return eval(col:c, row:r)
            }
            return (col:-2 * c, row:0)
        }
    ]

    if let eval = lookup[playerCount] {
        return eval
    }

    fatalError("wrong number of players: \(playerCount)")
}


gen(8)(col:0, row:1)
gen(16)(col:1, row:2)
gen(32)(col:3, row:0)

Slightly less verbose, but surely more ... entertaining? 😉

Oh, ok. I had misunderstood what you are trying to accomplish.

FWIW, besides the fact there is not what you want, maybe i should have written in this way...

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,numberOfPlayers) {
            case (0,_):
                return (newCol, row / 2)
            case (1,8), (2,16), (3,32):
                return (newCol, row ^ 1)
            case (1,16):
                return (newCol, 3 - row)
            case (1,32):
                return (newCol, (row + 4) % 8)
            case (2,32):
                return (newCol, row)
            default:
                return (newCol, 0)
            }
        }
    default:abort()
    }
}

But going back to you problem, so, if i understand well, it's a case of premature optimization, like in our example of colors:

func generateGetColorFunction(numberOfPlayers: Int) -> () -> UIColor {
    if numberOfPlayers == 8 {
        return {
            return UIColor.redColor()
        }
    } else if numberOfPlayers == 16 {
        return {
            return UIColor.blueColor()
        }
    } else if numberOfPlayers == 32 {
        return {
            return UIColor.greenColor()
        }
    } else {
        abort()
    }
}
let getColor = generateGetColorFunction(32)
let iAmGreen = getColor()

Should be written in swift in this way:

Considering colors and numberOfPlayers part of the same type and is immutable (like appear to be).

class SomeGame {
    let numberOfPlayers:Int
    lazy var color:UIColor = {
        switch self.numberOfPlayers {
            case 8:
                return UIColor.redColor()
            case 16:
                return UIColor.blueColor()
            case 32:
                return UIColor.greenColor()
            default:
                abort()
        }
    }();

    init(players: Int){
        numberOfPlayers = players;
    }
}

let game = SomeGame(players: 32);
let iAmGreen = game.color; //Compute once.
let colorGameInOtherContext = game.color // green, no switch again.

Using a lazy computed property will be better in this case, because the color is be computed once and will do not call UIColor.someColor() again. If the goal is premature optimization i think this solution is more reliable.

But if numberOfPlayers is not a imutable property of some part of SomeGame, and generateGetColorFunction or generateLoserLocationFunction its a global function its better you rethink you problem, and the verbose way its the only way...

For the fist case is the same, you need to use the "verbose way", or other creative way, and put in a lazy var location:(column: Int, row: Int) property. Will eliminate the inner return.

But for the practical purpose, switch numberOfPlayers like in my example above it's not expensive, you should not worry about that, probably its only simple 2 comparison more expensive the ideal (and verbose) case.

Great explanation about the lazy property!

LOL...just, LOL!

Not only does this seem like premature optimization, but I suspect that it's actively counterproductive.

When you structure the code using a function-returning function, you're preventing the compiler from doing any inlining. The compiler can't assume that theMethodToUse points anywhere in particular, because its value is calculated at run-time. So any reference to theMethodToUse() will probably result in an actual function call. I suspect you'd be better off from a performance standpoint doing an inlined switch on the number of players, even if that value is constant.

But you really can't say for sure without doing some testing. And regardless of the answer, this code is highly unlikely to ever be a performance bottleneck. You just shouldn't be worrying about performance in this context until you've been able to prove that it's an issue in the environment of the completed application.

As far as brevity, I don't think you can do much better than Wallacy's revised version with the tuple-valued switch. But that code does scramble up the cases in a way that makes it hard for a human to trace what's going on. I'd just keep the original structure:

private func loserLocation(nPlayers: Int, column: Int, row: Int) -> (column: Int, row: Int) {
    let newCol = -2 * column
    switch nPlayers {
        case 8:
            switch column {
                case 0:  return (newCol, row / 2)
                case 1:  return (newCol, row ^ 1)
                default: return (newCol, 0)
            }
        case 16:
            switch column {
                case 0:  return (newCol, row / 2)
                case 1:  return (newCol, 3 - row)
                case 2:  return (newCol, row ^ 1)
                default: return (newCol, 0)
            }
        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)
            }
        default:
            abort()
    }
}

It sure would be nice to collapse all those "return (newCol, ..." sections. Too bad switch isn't an expression in Swift.