An alternative would be to removedremove the current element from the input array:
An alternative would be to removed the current element from the input array:
An alternative would be to remove the current element from the input array:
My main point of criticism is that the code uses a global variable to share data between the main function and the auxiliary function. That is problematic for various reasons:
- The variable must be reset before the function can be called again.
- The variable can be modified from outside of your function, causing wrong results.
- The function is not thread-safe.
Example:
print(permsOfInteger(12)) // [[12], [21]]
print(permsOfInteger(12)) // [[12], [21], [12], [21]]
So
var outputArray = [[Int]]()
should be a local variable of func permsOfInteger()
. You can then pass it as an "inout argument" to the helper function func perms()
, or make that helper function a nested function of the main function:
func permsOfInteger(_ input: Int) -> [[Int]] {
var outputArray = [[Int]]()
func perms (_ input: [Int], _ output: [Int]) {
// ... adds something to `outputArray` ...
}
// ...
perms(inpArray, [])
return (outputArray)
}
Why is the return type a nested array where each "integer permutation" is wrapped into a single-element array? Returning a simple array of integers seems more natural to me, and would be more efficient:
func permsOfInteger(_ input: Int) -> [Int] {
// ...
}
print(permsOfInteger(12)) // [12, 21]
As @janos already said, combining the digits to an integer should be done in "pure maths" instead of using string operations. This can be done with reduce()
, for example:
let digits = [1, 2, 3]
let number = digits.reduce(0, { 0ドル * 10 + 1ドル })
print(number) // 123
An [UInt8]
array would be sufficient to store the decimal digits and save some memory. Here is a possible implementation as a computed property and custom initializer of the Int
type:
extension Int {
var decimalDigits: [UInt8] {
precondition(self >= 0, "Value must not be negative")
var digits = [UInt8]()
var n = self
while n > 0 {
digits.append(UInt8(n % 10))
n /= 10
}
return digits
}
init(decimalDigits: [UInt8]) {
self = decimalDigits.reduce(0, { 0ドル * 10 + Int(1ドル) })
}
}
The slicing
let before = Array(input[0..<i]) let after = Array(input[(i + 1) ..< input.count])
can be slightly shorted with partial ranges:
let before = Array(input[..<i])
let after = Array(input[(i + 1)...])
An alternative would be to removed the current element from the input array:
var remaining = input
let current = [remaining.remove(at: i)]
perms(remaining, current + output)
Instead of if input.count == 0
you can test if input.isEmpty
– it does not make a difference for arrays, but can be more efficient for arbitrary collections, where determining the count
requires a traversal of the entire collection.
For the same reason you might want to replace
for i in 0..<input.count { ... }
by
for i in input.indices { ... }
since collection indices are not always zero based.
Finally, I would suggest some more descriptive variable names and argument names. Putting it all together, the function could look like this:
func permutations(of number: Int) -> [Int] {
var permutations = [Int]()
func addPermutations(of digits: [UInt8], withSuffix suffix: [UInt8]) {
if digits.isEmpty {
permutations.append( Int(decimalDigits: suffix) )
} else {
for i in digits.indices {
var remainingDigits = digits
let currentDigit = [remainingDigits.remove(at: i)]
addPermutations(of: remainingDigits, withSuffix: currentDigit + suffix)
}
}
}
addPermutations(of: number.decimalDigits, withSuffix: [])
return (permutations)
}
print(permutations(of: 123))
// [123, 213, 132, 312, 231, 321]