trjhtr

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.

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'