General remarks
The computation should be separated from the program output. That increases the clarity of the program, and makes the functions better testable. In your case, func playertold
should return the result instead of printing it.
Type annotations are often not needed, e.g.
var currentNumber:Int = 1
var currentPlayer:Int = 1
var reversedOrder:Bool = false
can be simplified to
var currentNumber = 1
var currentPlayer = 1
var reversedOrder = false
The indending and spacing it not consistent: The function body and some blocks are not indented, some empty lines are superfluous.
Simplify the program logic
The while-loop
var currentNumber:Int = 1
while currentNumber != number {
// ... do something ...
currentNumber += 1
}
can be simplified to a for-loop over a range:
for currentNumber in 1..<number {
// ... do something ...
}
Instead of a boolean variable reversedOrder
you can use a integer variable direction
which holds the values +1
or -1
. Advancing to the next player is then done with
currentPlayer += direction
// or:
currentPlayer += 2 * direction
depending on whether the next player is skipped or not. After updating the currentPlayer
one can check for wrap-around. That saves a lot of case distinctions.
Instead of the remainder operator %
can can use the isMultiple(of:)
method which was introduced in Swift 5, which expresses the intent better.
The function would then look like this:
func playertold(number: Int) -> Int {
var currentPlayer = 1
var direction = +1
for currentNumber in 1..<number {
if currentNumber.isMultiple(of: 9) {
direction = -direction
}
// Advance position:
if currentNumber.isMultiple(of: 17) {
currentPlayer += 2 * direction
} else {
currentPlayer += direction
}
// Check for wrap-around:
if currentPlayer < 1 {
currentPlayer += 17
} else if currentPlayer > 17 {
currentPlayer -= 17
}
}
return currentPlayer
}
More suggestions
Avoid “magic constants.” In this function that are the numbers 9 and 17, and the latter has two different meanings. What if you want to change the number of players from 17 to 20? In your original code you would have to replace multiple instances of 16 and 17, and in the above version you still have to multiple instances of 17 – but not the 17 in the isMultiple(of: 17)
check! That is too error-prone.
One option is to define constants, e.g.
let numPlayers = 17
let reversePosition = 9
let skipPosition = 17
Another – more flexible – option is to make these additional parameters of the function, with default values:
func playertold(number: Int, numPlayers: Int = 17,
reversePosition: Int = 9, skipPosition: Int = 17) -> Int {
var currentPlayer = 1
var direction = +1
for currentNumber in 1..<number {
if currentNumber.isMultiple(of: reversePosition) {
direction = -direction
}
// Advance position:
if currentNumber.isMultiple(of: skipPosition) {
currentPlayer += 2 * direction
} else {
currentPlayer += direction
}
// Check for wrap-around:
if currentPlayer < 1 {
currentPlayer += numPlayers
} else if currentPlayer > numPlayers {
currentPlayer -= numPlayers
}
}
return currentPlayer
}
Now the intent of every constant is clear. The function can still be called with
the standard values
print(playertold(number: 1234))
but also with custom values
print(playertold(number: 1234, numPlayers: 20, reversePosition: 5, skipPosition: 15))