HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

Implementing joinWithSeparator on a Character array

Submitted by: @import:stackexchange-codereview··
0
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 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 separator: String parameter, but actually it is
expected 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 cluster


The 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 it
actually works only for Arrays. For arbitrary sequences of characters,
the optional cast as? [Character] fails. This is silently ignored
and the separator not inserted:

let charSeq = Repeat(count: 4, repeatedValue: Character("x"))
let joined = charSeq.joinWithSeparator(",")
print(joined) // Output: xxxx


Two 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 be
opinion 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 cluster
let charSeq = Repeat(count: 4, repeatedValue: Character("x"))
let joined = charSeq.joinWithSeparator(",")
print(joined) // Output: xxxx
self.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.