patternswiftMinor
Implementing joinWithSeparator on a Character array
Viewed 0 times
arrayimplementingcharacterjoinwithseparator
Problem
I recently answered a Stack Overflow question about transforming an array of Characters into a String in Swift. Looking at the question and the Swift standard lib, it appears that there is a method
How can I further optimize this code? Would it be possible to replace the
joinWithSeparator, but the current implementation only supports SequenceType instances where the Element is a String. As an exercise to myself, I wanted to write an extension on SequenceType that could flatten an array of Characters into a String:extension SequenceType where Generator.Element == Character {
@warn_unused_result
public func joinWithSeparator(separator: String) -> String {
var str = ""
self.enumerate().forEach({
str.append($1)
if let arr = self as? [Character], endIndex: Int = arr.endIndex {
if $0 < endIndex - 1 {
str.append(Character(separator))
}
}
})
return str
}
}How can I further optimize this code? Would it be possible to replace the
forEach loop with flatMap? Or is a map call inappropriate since I need to append each character to a String, rather than create a new array?Solution
There are two problems with your method:
Problem #1: The method takes a
expected that a single character string is passed, and it crashes
at
The solution is easy: change the parameter to
Btw, this makes the method faster.
Problem #2: The method is an extension of
actually works only for
the optional cast
and the separator not inserted:
Two more remarks before I suggest possible solutions:
-
The name of the local variable
I would call it
opinion based).
-
Your loop
is correct, but I would write it as
So how can problem #2 be solved? You cast the sequence to an array in
order to get the index of the last element, so that you can insert the
separator after all elements but the last.
It becomes simpler if you think the other way around: Insert the
separator before all elements but the first. No need to determine
the
This has about the same speed as your method when applied to an
array, but works for arbitrary character sequence.
You asked:
Would it be possible to replace the forEach loop with flatMap?
and the answer is "yes":
The shorted implementation (but not the fastest) would be to convert all characters
to strings, and use the existing method to join strings:
Performance comparison
Here is my test code (compiled in Release
mode on a 3.5Ghz iMac):
Results:
Problem #1: The method takes a
separator: String parameter, but actually it isexpected that a single character string is passed, and it crashes
at
Character(separator) if a multi-character string is passed:let myChars: [Character] = ["a", "b", "c"]
let joined = myChars.joinWithSeparator("==")
print(joined)
// fatal error: Can't form a Character from a String containing more than one extended grapheme clusterThe solution is easy: change the parameter to
separator: Character.Btw, this makes the method faster.
Problem #2: The method is an extension of
SequenceType, but itactually works only for
Arrays. For arbitrary sequences of characters,the optional cast
as? [Character] fails. This is silently ignoredand the separator not inserted:
let charSeq = Repeat(count: 4, repeatedValue: Character("x"))
let joined = charSeq.joinWithSeparator(",")
print(joined) // Output: xxxxTwo more remarks before I suggest possible solutions:
-
The name of the local variable
var str = "" is quite non-descriptive.I would call it
result or joinedString (but that may beopinion based).
-
Your loop
self.enumerate().forEach {
/* do something with `$0` (index) and `$1` (char)
}is correct, but I would write it as
for (index, char) in self.enumerate() {
/* do something with `index` and `char`
}So how can problem #2 be solved? You cast the sequence to an array in
order to get the index of the last element, so that you can insert the
separator after all elements but the last.
It becomes simpler if you think the other way around: Insert the
separator before all elements but the first. No need to determine
the
endIndex anymore:public func joinWithSeparator(separator: Character) -> String {
var result = ""
for (idx, char) in self.enumerate() {
if idx > 0 {
result.append(separator)
}
result.append(char)
}
return result
}This has about the same speed as your method when applied to an
array, but works for arbitrary character sequence.
You asked:
Would it be possible to replace the forEach loop with flatMap?
and the answer is "yes":
public func joinWithSeparator(separator: Character) -> String {
let joinedChars = self.enumerate().flatMap {
(idx, char) in
idx == 0 ? [ char ] : [ separator, char ]
}
return String(joinedChars)
}The shorted implementation (but not the fastest) would be to convert all characters
to strings, and use the existing method to join strings:
public func joinWithSeparator(separator: Character) -> String {
return self.map { String($0) }.joinWithSeparator(String(separator))
}Performance comparison
Here is my test code (compiled in Release
mode on a 3.5Ghz iMac):
let myChars = Array(Repeat(count: 1_000_000, repeatedValue: Character("x")))
let d1 = NSDate()
let j = myChars.joinWithSeparator(",")
let d2 = NSDate()
print(d2.timeIntervalSinceDate(d1))Results:
- Your original code: 0.163 sec.
- Your code, with the separator parameter changed to
Character: 0.104 sec.
- My first suggestion: 0.095 sec.
- Using flatMap: 0.276 sec.
- Convert all characters to strings: 0.305 sec.
Code Snippets
let myChars: [Character] = ["a", "b", "c"]
let joined = myChars.joinWithSeparator("==")
print(joined)
// fatal error: Can't form a Character from a String containing more than one extended grapheme clusterlet charSeq = Repeat(count: 4, repeatedValue: Character("x"))
let joined = charSeq.joinWithSeparator(",")
print(joined) // Output: xxxxself.enumerate().forEach {
/* do something with `$0` (index) and `$1` (char)
}for (index, char) in self.enumerate() {
/* do something with `index` and `char`
}public func joinWithSeparator(separator: Character) -> String {
var result = ""
for (idx, char) in self.enumerate() {
if idx > 0 {
result.append(separator)
}
result.append(char)
}
return result
}Context
StackExchange Code Review Q#133510, answer score: 5
Revisions (0)
No revisions yet.