Swift function to interpret Roman numerals (ported from JavaScript)

Question

I have written some Swift for the first time, being competent enough in Javascript and having some experience in Ruby and Python. For my education, I've written a function that parses a roman numeral string and returns its integer representation, first in Javascript (ES2015+):

const dict = [
  ['CM', 900], ['M', 1000], ['CD', 400], ['D', 500],
  ['XC', 90], ['C', 100], ['XL', 40], ['L', 50],
  ['IX', 9], ['X', 10], ['IV', 4], ['V', 5],
  ['I', 1],
]

function romanToInt (original) {
  let temp = original
  let int = 0

  while (temp.length > 0) {
    let found = false

    for (const [glyph, quantity] of dict) {
      if (temp.startsWith(glyph)) {
        int += quantity
        temp = temp.slice(glyph.length)
        found = true
        break
      }
    }

    if (!found) {
      // e.g. Error parsing roman numeral "MDCCLXXVI," at ","
      throw new Error(`Error parsing roman numeral "${original}" at "${temp}"`)
    }
  }

  return int
}

try {
  romanToInt('MMXIV') // => 2014
} catch (err) {
  console.error(err)
}

and then ported it to Swift 4:

let dict: [( glyph: String, quantity: Int )] = [
    ("CM", 900), ("M", 1000), ("CD", 400), ("D", 500),
    ("XC", 90), ("C", 100), ("XL", 40), ("L", 50),
    ("IX", 9), ("X", 10), ("IV", 4), ("V", 5),
    ("I", 1)
]

enum RomanNumericError: Error {
    case badInput(original: String, temp: String)
}

func romanToInt(original: String) throws -> Int {
    var temp = original
    var int = 0

    while temp.count > 0 {
        var found = false

        for (glyph, quantity) in dict {
            if temp.starts(with: glyph) {
                int += quantity
                temp.removeFirst(glyph.count)
                found = true
                break
            }
        }

        guard found == true else {
            throw RomanNumericError.badInput(original: original, temp: temp)
        }
    }

    return int
}

do {
    try romanToInt(original: "MMXIV") // => 2014
} catch RomanNumericError.badInput(let original, let temp) {
    print("Error parsing roman numeral '\(original)' at '\(temp)'")
}

I'm wondering about how swift-y my code is in terms of design patterns, especially in terms of error handling. In Javascript, throwing and catching errors is a very common control flow design, and I'm wondering if I'm approaching it from the right angle in Swift.

Answer 1

Let's do this from inside out. temp.starts(with: glyph) is correct, but there is also a dedicated method temp.hasPrefix(glyph) for strings.

The loop to find the first dictionary entry with a matching prefix can be shortened to

guard let (glyph, quantity) = dict.first(where: { temp.hasPrefix($0.glyph) }) else {
    throw RomanNumericError.badInput(original: original, temp: temp)
}

(also making the var found obsolete.)

Mutating the temporary string can be avoided by working with a SubString (which is a kind of view into the original string) and only updating the current search position:

var pos = original.startIndex
while pos != original.endIndex {
    let subString = original[pos...]
    // ...
    pos = original.index(pos, offsetBy: glyph.count)
}

Naming: This is very opinion-based, here are my opinions:

• Declare the function as func romanToInt(_ roman: String), so that it is called without (external) argument name: romanToInt("MMXIV").
• Rename var int to var value.
• dict is also a non-descriptive name (and it is not even a dictionary), something like glyphsAndValues might be a better choice.

Summarizing the suggestions so far, we have

func romanToInt(_ roman: String) throws -> Int {
    var value = 0
    var pos = roman.startIndex
    while pos != roman.endIndex {
        let subString = roman[pos...]
        guard let (glyph, quantity) = glyphsAndValues.first(where: { subString.hasPrefix($0.glyph) }) else {
            throw RomanNumericError.badInput(roman: roman, at: subString)
        }
        value += quantity
        pos = roman.index(pos, offsetBy: glyph.count)
    }
    return value
}

---

Now the error handling. Yes, throwing an error is a good and Swifty way to report a failure to the caller. (An alternative is to return an optional value which is nil in the error case, but that does not allow to provide additional error information.)

The creation of the error message however should be done in the error class, by adopting the LocalizedError protocol:

enum RomanNumericError: Error {
    case badInput(roman: String, at: Substring)
}

extension RomanNumericError: LocalizedError {
    public var errorDescription: String? {
        switch self {
        case .badInput(let roman, let at):
            return "Error parsing roman numeral '\(roman)' at '\(at)'"
        }
    }
}

The big advantage is that the caller does not need to know which error the function throws, and can catch a generic Error:

do {
    try print(romanToInt("MMXIV"))
    try print(romanToInt("MMYXIV"))
} catch {
    print(error.localizedDescription)
}

// Output:
//   2014
//   Error parsing roman numeral 'MMYXIV' at 'YXIV'