Model for math facts

Question

I've written a model for an app that generates math facts kids have to solve by evaluating the operation and the difficulty you pass to it at initialization.

I don't like how I am repeating the code that performs the operations. For instance, performAddition() and performMultiplication() both perform the same thing. How can I make this model better?

import UIKit

class QuestionGenerator: NSObject {

    enum BinaryOperation {
        case Addition
        case Subtraction
        case Multiplication
        case Division
    }

    enum Difficulty {
        case Easy
        case Intermediate
        case Difficult
    }

    struct Settings {
        var binaryOperation: BinaryOperation
        var difficulty: Difficulty
    }

    private var brain: Settings
    var number1: Int = 0
    var number2: Int = 0
    var answer: Int = 0

    init(operation: BinaryOperation, difficulty: Difficulty) {
        brain = Settings(binaryOperation: operation, difficulty: difficulty)
        super.init()
    }

    func newQuestion() {
        switch brain.binaryOperation {
        case .Addition:
            performAddition()
            break
        case .Multiplication:
            performMultiplication()
            break
        default: break
            // operation not implemented yet
        }
    }

    private func performAddition() {
        switch brain.difficulty {
        case .Easy:
            number1 = Int.random(1...10)
            number2 = Int.random(1...10)
        case .Intermediate:
            number1 = Int.random(10...100)
            number2 = Int.random(10...100)
        case .Difficult:
            number1 = Int.random(109...999)
            number2 = Int.random(109...999)
        }
        answer = number1 + number2
    }

    private func performMultiplication() {
        switch brain.difficulty {
        case .Easy:
            number1 = Int.random(1...10)
            number2 = Int.random(2...4)
        case .Intermediate:
            number1 = Int.random(1...50)
            number2 = Int.random(3...7)
        case .Difficult:
            number1 = Int.random(10...100)
            number2 = Int.random(4...15)
        }
        answer = number1 * number2
    }
}

Answer 1

Disclaimer, I'm a beginner level Swift programmer, so take my feedback with a grain portion of salt.

First off, a minor improvement for your newQuestion function, you can leave out the breaks, in Swift, cases don't fall through by default:

func newQuestion() {
    switch brain.binaryOperation {
    case .Addition:
        performAddition()
    case .Multiplication:
        performMultiplication()
    default: break
        // operation not implemented yet
    }
}

---

Secondly, I see no reason you need to put binaryOperation and difficulty into a Settings struct. Furthermore, from what I can see, they should be final. I'd suggest you rewrite thing to:

private let binaryOperation: BinaryOperation
private let difficulty: Difficulty

...

init(operation: BinaryOperation, difficulty: Difficulty) {
    self.binaryOperation = operation
    self.difficulty = difficulty
    super.init()
}

---

Then, to generalise your perform functions, I'd add two functions to your BinaryOperation enum:

enum BinaryOperation {
    case Addition
    case Subtraction
    case Multiplication
    case Division

    func rangesForDifficulty(difficulty: Difficulty) -> (Range<Int>, Range<Int>) {
        switch self {
        case .Addition:
            switch difficulty {
            case .Easy: return (1...10, 1...10)
            case .Intermediate: return (10...100, 1...100)
            case .Difficult: return (109...999, 109...999)
            }
        case .Subtraction:
            switch difficulty {
            case .Easy: return (1...10, 1...10)
            case .Intermediate: return (10...100, 1...100)
            case .Difficult:  return (109...999, 109...999)
            }
        case .Multiplication:
            switch difficulty {
            case .Easy: return (1...10, 2...4)
            case .Intermediate: return (1...50, 3...7)
            case .Difficult: return (10...100, 4...15)
            }
        case .Division:
            switch difficulty {
            case .Easy: return (1...10, 2...4)
            case .Intermediate: return (1...50, 3...7)
            case .Difficult: return (10...100, 4...15)
            }
        }
    }

    func apply(number1: Int, _ number2: Int) -> Int {
        switch self {
        case .Addition:
            return number1 + number2
        case .Subtraction:
            return number1 - number2
        case .Multiplication:
            return number1 * number2
        case .Division:
            return number1 / number2
        }
    }
}

This allows you to write a single the newQuestion function much more elegantly:

func newQuestion() {
    let (range1, range2) = binaryOperation.rangesForDifficulty(difficulty)
    number1 = Int.random(range1)
    number2 = Int.random(range2)
    answer = binaryOperation.apply(number1, number2)
}

---

Some people might argue, that the BinaryOperation enum has gotten unwieldy now and they might be right. You could consider putting the rangesForDifficulty function somewhere else to alleviate this, I do think that the apply function should stay where it is though.