Rewrite Dice parsing

[Samasaur1]

DiceKit/Sources/DiceKit/Dice.swift

Lines 168 to 252 in d67f52f

	//swiftlint:disable function_body_length
	/// Creates a new `Dice` object from the specified string in dice notation.
	///
	/// You cannot have a negative die **AS A RESULT** (`-d6`), a die with negative sides (`d-6`), or a die with 0 sides (`d0`). You cannot have an unreal modifier or use any operator except for addition and subtraction.
	///
	/// You can have `-d6`s in your string, so long as they cancel each other out so that the final result is at least `0d6`.
	///
	/// - Parameter str: The string to convert.
	/// - Throws: An `Error.IllegalNumberOfSides` error when the number of sides is less than or equal to 0
	public init(_ str: String) throws {
	var dice: [Int: Int] = [:]
	var mods: [Int] = []
	let str = str.filter({ $0 != " " })
	guard Set(str).isSubset(of: ["0", "1", "2", "3", "4", "5", "6", "7", "8", "9", "+", "-", "D", "d"]) else {
	throw Error.illegalString(string: str)
	}
	let plusSplit = str.split(whereSeparator: { $0 == "+" })
	for positiveExp in plusSplit {
	let exp = positiveExp.split(whereSeparator: { $0 == "-" })
	if positiveExp.starts(with: "-") {
	for ex in exp {//negative
	if String(ex).isNumeric {
	mods.append(-Int(String(ex))!)
	} else if ex.contains("D") || ex.contains("d") {
	let d = ex.split(whereSeparator: { $0 == "d" || $0 == "D" })
	if d.count == 1 {
	let sides = Int(String(d.first!))!
	dice[sides] = (dice[sides] ?? 0) - 1
	} else if d.count == 2 {
	let multiplier = Int(String(d.first!))!
	let sides = Int(String(d.last!))!
	dice[sides] = (dice[sides] ?? 0) - multiplier
	} else { throw Error.illegalString(string: str) } //Too many "d"s (e.g. 3d4d6)
	} else { throw Error.illegalString(string: str) } //non-numeric and not a d, so...
	}
	} else {
	let ex = String(exp.first!)//positive
	if ex.isNumeric {
	mods.append(Int(ex)!)
	} else if ex.contains("D") || ex.contains("d") {
	let d = ex.split(whereSeparator: { $0 == "d" || $0 == "D" })
	if d.count == 1 {
	let sides = Int(String(d.first!))!
	dice[sides] = (dice[sides] ?? 0) + 1
	} else if d.count == 2 {
	let multiplier = Int(String(d.first!))!
	let sides = Int(String(d.last!))!
	dice[sides] = (dice[sides] ?? 0) + multiplier
	} else { throw Error.illegalString(string: str) } //Too many "d"s (e.g. 3d4d6)
	} else { throw Error.illegalString(string: str) } //non-numeric and not a d, so...
	for ex in exp.dropFirst() {//negative
	if String(ex).isNumeric {
	mods.append(-Int(String(ex))!)
	} else if ex.contains("D") || ex.contains("d") {
	let d = ex.split(whereSeparator: { $0 == "d" || $0 == "D" })
	if d.count == 1 {
	let sides = Int(String(d.first!))!
	dice[sides] = (dice[sides] ?? 0) - 1
	} else if d.count == 2 {
	let multiplier = Int(String(d.first!))!
	let sides = Int(String(d.last!))!
	dice[sides] = (dice[sides] ?? 0) - multiplier
	} else { throw Error.illegalString(string: str) } //Too many "d"s (e.g. 3d4d6)
	} else { throw Error.illegalString(string: str) } //non-numeric and not a d, so...
	}
	}
	}
	var tempDice: [Die: Int] = [:]
	for (d, c) in dice {
	if c < 0 {
	throw Error.illegalString(string: str)
	} else if c == 0 {
	continue
	}
	tempDice[try Die(sides: d)] = c
	}
	self.dice = tempDice
	self.modifier = mods.sum
	}
	//swiftlint:enable function_body_length

So just by looking at the code for the string parsing of dice expressions, I think it should be pretty clear that it’s not a great implementation. To be fair, it does pass my fairly comprehensive set of unit tests, but I still think that a) that’s probably due to luck; b) even if it works, it’s not maintainable.

I’d like to replace it with recursive-descent parsing (or really, any structured type of parsing, but I have experience with recursive-descent, and I think it would work with this scenario.