Anchor Text String Transformation in Swift

Question

I need to convert a string using these rules:

• Downcase the string
• Replace spaces, a blacklist of "invalid" chars, and non-ascii letters (like é) with -
• Replace repeated hyphens (ie --) with one hyphen (-)

For example, Is my résumé good enough? should convert to is-my-r-sum-good-enough-.

This Swift function converts the string. I'm looking for help making it shorter. Also, if there's a way to avoid import Foundation (by replacing containsString and stringByReplacingOccurrencesOfString), I'd like to do that. I know I could use an NSRegularExpression but I don't want to use Foundation classes.

Here's what I have so far:

extension Character {
    static let invalidSet = Set(" -&+$,/:;=?@\"#{}|^~[]`\\*()%.!'".characters)

    var invalid: Bool {
        return Character.invalidSet.contains(self)
    }

    var isAscii: Bool {
        guard let number = String(self).utf16.first else { return false }
        return (65..<127) ~= number
    }
}

func anchor(header: String) -> String {
    var output = String(header.lowercaseString.characters.map {
        $0.invalid || !$0.isAscii ? "-" : $0
    })

    while output.containsString("--") {
        output = output.stringByReplacingOccurrencesOfString("--", withString: "-")
    }

    return output
}

Usage:

let input = "Is my résumé good enough?"
let expectedResult = "is-my-r-sum-good-enough-"

print(anchor(input)) //prints is-my-r-sum-good-enough-
print(anchor(input) == expectedResult ? "😀" : "😞") //prints 😀

Answer 1

We should always be thinking about how to write our code in a generic way. Otherwise, what happens if we want something very similar to what we've just written, but slightly different in some minor way? Well, it usually results in a lot of copy & pasting. Let's write some code that will allow us to apply the rules you want to apply, but in a more generic way.

I want to start with a skeleton that looks something like this:

struct StringFormatter {
    enum CaseRule {
        case None, UppercaseOnly, ConvertToUpper, LowercaseOnly, ConvertToLower
    }

    enum AsciiRule {
        case None, AsciiOnly
    }

    var blacklistCharacters = Set<Character>()
    var replacementCharacter = ""
    var caseRule = CaseRule.None
    var asciiRule = AsciiRule.None

    func stringByApplyingFormatting(toString string: String) -> String {
        // TODO: Implement actual formatting logic
        return ""
    }
}

Now we've got a reusable structure for applying this sort of formatting to our strings. Importantly, despite there being no logic in it yet, we have implemented the method we'd be calling to apply the formatting, so we can stick with test-driven development and go ahead and write our unit test.

class StringTestStuff: XCTestCase {
    func testStringFormatting() {
        let input = "Is my résumé good enough?"
        let expectedResult = "is-my-r-sum-good-enough-"

        var stringFormatter = StringFormatter()
        stringFormatter.blacklistCharacters = Set(" -&+$,/:;=?@\"#{}|^~[]`\\*()%.!'".characters)
        stringFormatter.replacementCharacter = "-"
        stringFormatter.caseRule = .ConvertToLower
        stringFormatter.asciiRule = .AsciiOnly

        let formattedString = stringFormatter.stringByApplyingFormatting(toString: input)

        XCTAssertEqual(formattedString, expectedResult)
    }
}

We can run the test and see that's it's failing (confirming that the test works):

Of course, much like my previous answer, this test is far from complete. All this test is doing, though, is simply testing the one known acceptance criteria you've provided. Realistically, we'd want quite a few tests for this, and we'd want to test several of the different aspects of the StringFormatter individually (applying just the case rule, applying just the ASCII rule, etc), and several variations of different combinations. We'd also probably want to see some performance tests for the StringFormatter too. There are going to be multiple ways to accomplish this, and we want to make sure that when we refactor, we've not only not broken things, but we've also not made things run super slow.

But let's not kid ourselves, it would be impossible to ever test this code nearly as completely as the simply isAscii test manages. We will simply have to pick a set of smart tests and be prepared to add tests as edge cases appear.

So with all this in mind, let's work on making this test pass. (I know your code would pass the test, but let's see if we can do it without import Foundation).

In order to keep in line with the single responsibility principle, we need to add a couple of helper functions.

First, this function will apply our caseRule property:

private func stringWithCaseConversionApplied(string: String) -> String {
    switch caseRule {
    case .ConvertToLower: return string.lowercaseString
    case .ConvertToUpper: return string.uppercaseString
    default: return string
    }
}

And if our caseRule property is set to an "only" value rather than a "convert" value, this method will determine if any individual character would pass that check:

private func caseFilter(character: Character) -> Bool {
    if caseRule == .LowercaseOnly && String(character).uppercaseString == String(character) {
        return false
    }

    if caseRule == .UppercaseOnly && String(character).lowercaseString == String(character) {
        return false
    }

    return true
}

But we've implemented another method to apply all of the filtering rules:

private func ruleFilter(character: Character) -> Bool {
    if blacklistCharacters.contains(character) {
        return false
    }

    if asciiRule == .AsciiOnly && !character.isAscii {
        return false
    }

    return caseFilter(character)
}

So, now we have all the pieces in place to check individual characters. It's time to break out some of the Swift standard library functions that will allow us to avoid importing Foundation.

We're back to working in stringByApplyingFormatting(), the method around which we've written our unit test.

We're going to first apply the case conversion rule by calling the method we wrote to do that, then we're going to filter out any illegal characters based on our ruleFilter method. This will break our string down into an array of character sequences which can be joined back together with our replacementCharacter. This will eliminate the trouble of repeatedly checking for occurrences of doubles of the replacement character.

The last step looks a little weird. We need to check if the first or last character was an illegal character, and if so, stick the replacement character at the beginning or end (or both) because the join method will only place it between our sub sequences. It won't keep track of whether or not it eliminated the first or last characters.

So, the logic within our stringByApplyingFormatting() method now looks like this:

func stringByApplyingFormatting(toString string: String) -> String {
    // Apply case conversion rule
    let caseValue = stringWithCaseConversionApplied(string)

    // Apply the rule filter to each character
    // This splits the string into an array of character subsequences
    let splitCharacters = caseValue.characters.split { !ruleFilter($0) }

    // Map the array of character subsequences to an array of strings that can be joined
    let splitString = splitCharacters.map(String.init)

    // Join the array of strings with the specified replacement character
    let joinedString = splitString.joinWithSeparator(replacementCharacter)

    // Check if the original string started or ended with an illegal character
    // If so, prepend/append the replacement character
    let finalString = 
        (!ruleFilter(caseValue.characters.first ?? Character("")) ? replacementCharacter : "") + 
        joinedString + 
        (!ruleFilter(caseValue.characters.last ?? Character("")) ? replacementCharacter : "")

    return finalString
}

And with this logic, if we re-run our unit test, it's passing now. And we also no longer need to import Foundation into our code base, so this code runs perfectly well on Linux too.

From here, the next step is adding a pretty significant suite of tests around all of the individual rules and all of the possible combinations of rules, with all sorts of test strings.

---

The full StringFormatter struct:

struct StringFormatter {
    enum CaseRule {
        case None, UppercaseOnly, ConvertToUpper, LowercaseOnly, ConvertToLower
    }

    enum AsciiRule {
        case None, AsciiOnly
    }

    var blacklistCharacters = Set<Character>()
    var replacementCharacter = ""
    var caseRule = CaseRule.None
    var asciiRule = AsciiRule.None

    func stringByApplyingFormatting(toString string: String) -> String {
        let caseValue = stringWithCaseConversionApplied(string)

        let splitCharacters = caseValue.characters.split { !ruleFilter($0) }

        let splitString = splitCharacters.map(String.init)

        let joinedString = splitString.joinWithSeparator(replacementCharacter)

        let finalString = (!ruleFilter(caseValue.characters.first ?? Character("")) ? replacementCharacter : "") + joinedString + (!ruleFilter(caseValue.characters.last ?? Character("")) ? replacementCharacter : "")

        return finalString
    }

    private func stringWithCaseConversionApplied(string: String) -> String {
        switch caseRule {
        case .ConvertToLower: return string.lowercaseString
        case .ConvertToUpper: return string.uppercaseString
        default: return string
        }
    }

    private func ruleFilter(character: Character) -> Bool {
        if blacklistCharacters.contains(character) {
            return false
        }

        if asciiRule == .AsciiOnly && !character.isAscii {
            return false
        }

        return caseFilter(character)
    }

    private func caseFilter(character: Character) -> Bool {
        if caseRule == .LowercaseOnly && String(character).uppercaseString == String(character) {
            return false
        }

        if caseRule == .UppercaseOnly && String(character).lowercaseString == String(character) {
            return false
        }

        return true
    }
}

The unit test is the same as posted above:

class StringTestStuff: XCTestCase {
    func testStringFormatting() {
        let input = "Is my résumé good enough?"
        let expectedResult = "is-my-r-sum-good-enough-"

        var stringFormatter = StringFormatter()
        stringFormatter.blacklistCharacters = Set(" -&+$,/:;=?@\"#{}|^~[]`\\*()%.!'".characters)
        stringFormatter.replacementCharacter = "-"
        stringFormatter.caseRule = .ConvertToLower
        stringFormatter.asciiRule = .AsciiOnly

        let formattedString = stringFormatter.stringByApplyingFormatting(toString: input)

        XCTAssertEqual(formattedString, expectedResult)
    }
}

---

As an addendum, one thing that might be neat to add to the StringFormatter struct would be a means for mapping certain characters to be replaced with specified characters. So, for example, you're not going to allow ASCII, but you could potentially replace è with a "regular" e.

So we might add a property to the struct that looks like this:

var replacementMap = [Character: Character]()

So it's a dictionary where the key is the character we want to remove and the value is the character we want to replace it with.

stringFormatter.replacementMap = [
    "è" : "e",
    "é" : "e",
    "ê" : "e",
    "ë" : "e",
    "ē" : "e",
    "ė" : "e",
    "ę" : "e"
]

Of course, that particular map just covers the characters that might map to "e", and we'd have to do this before we applied the ASCII rule. But implementation would look something like this:

private func stringByApplyingReplacementMap(string: String) -> String {
    return String(string.characters.map { return self.replacementMap[$0] ?? $0 })
}

---

For more reading on testing, check out Unit Tests Don't Prevent Bugs.

Answer 2

You've made very strange decisions as to what belongs in extensions of existing types and what does not. By my measure, the only correct decision was to include isAscii as a property of the Character type, however, your implementation is incorrect.

Your isAscii only returns true for 62 of the 128 ASCII characters. It's more important that our code does what it says it does than for it to do what we need it to do. Right now, your isAscii returns false for numbers and has inconsistent behavior for symbols and non-printable characters (the delete character returns true while all the other non-printables return false). That might be what we need today, but in 6 months, you might need something that actually gives the correct result on whether or not a character is ASCII or not.

So if we're going to call something isAscii, it should behave exactly as the most common sense interpretation of that method name would have you believe it behaves. If it does not, you will say "Hmm, this method has a bug." And you'll fix that bug. But in so fixing that bug, there is an unknown amount of code that was written in the meantime that may have had bugs introduced by your fix because the code was relying on this incorrect isAscii's implementation, which returns false for numbers (but is now suddenly returning true since it has been fixed).

If we want a function that only returns true for letters that are in the ASCII range, then we'd want something more like: isAsciiLetter. But it should be noted that even with this name, your code still needs fixing, because it would still be incorrectly returning true for the non-printable delete character, as well as a handful of symbols.

And for reference, isAsciiLetter could look something like this:

extension Character {
    var isAsciiLetter: Bool {
        return "A"..."Z" ~= self || "a"..."z" ~= self
    }
}

Before we start trying to fix the method, let's write a unit test that will, at a minimum, make sure that we're returning true for all 128 ASCII characters. That test would look something like this:

class CharacterStuffTest: XCTestCase {
    func testIsAscii() {
        for scalar in 0...127 {
            let testCharacter = Character(UnicodeScalar(scalar))
            XCTAssertTrue(testCharacter.isAscii, "Testing \(testCharacter) which is ASCII")
        }
    }
}

And if we run that now, with your current implementation, we get 66 failures.

Of course, this test isn't necessarily complete. This test only verifies that we are returning true for all characters in ASCII. It does nothing to verify we're returning false for everything else. So, do you want to write a test for that?

That test might look something like this:

class CharacterStuffTest: XCTestCase {
    func testIsAscii() {
        let asciiRange = 0...127
        let utf16Range = 0...32767
        for scalar in utf16Range {
            let testCharacter = Character(UnicodeScalar(scalar))
            XCTAssertEqual(asciiRange ~= scalar, testCharacter.isAscii, "Testing \(testCharacter)")
        }
    }
}

Be careful with this test though. It runs perfectly fine, but it's running 32,767 tests, and if there are a lot of failures, Xcode struggles to report them all. Fortunately, the current implementation of your isAscii is only failing within the ASCII range. But we still want to run our test well outside the ASCII range to make sure we're not incorrectly reporting non-ASCII characters as ASCII.

So, with a more complete understand of what exactly constitutes an ASCII character, fixing your original function up should be fairly easy. We just need to include all of the right values in the range.

extension Character {
    var isAscii: Bool {
        return Character(UnicodeScalar(0))...Character(UnicodeScalar(127)) ~= self
    }
}

As a note, we can actually replace utf16 with utf8 and the function still works perfectly fine, but it seems to run slightly slower (probably to do with optionals and branching).

---

This is a more complete test:

func testIsAscii() {
    let testUnicode = { (unicode: Int) in
        let asciiRange = 0...127
        let testCharacter = Character(UnicodeScalar(unicode))
        XCTAssertEqual(asciiRange ~= unicode, testCharacter.isAscii, "Testing \(testCharacter)")
    }

    var checkedCharacters = 0

    let utf16Range1 = 0...55295
    for scalar in utf16Range1 {
        testUnicode(scalar)
        checkedCharacters += 1
    }

    let utf16Range2 = 57344...1114111
    for scalar in utf16Range2 {
        testUnicode(scalar)
        checkedCharacters += 1
    }

    print("Checked \(checkedCharacters) characters.")
}