Basic calculator in Swift 3

Question

I just started learning the basics to Swift 3 about 2 days ago. Since then, I've decided to make a simple calculator that can only do one operation at a time.

I was wondering how efficient this was and what I could possibly do to make it more efficient. Any type of criticism would be helpful too

import UIKit
class ViewController: UIViewController {
 @IBOutlet var buttonResponseLabel: UILabel!
 var userIsCurrentlyTyping: Bool = false
 var storedValue: Int = 0;
 var currentOperation = 0
 var currentNumberInLabel = ""
 @IBAction func buttonAppend(_ sender: UIButton)
 {
 let buttonTitle = sender.currentTitle!
 switch buttonTitle
 {
 case "+":
 currentOperation = 1
 storedValue = Int(currentNumberInLabel)!
 buttonResponseLabel.text = ""
 case "-":
 currentOperation = 2
 storedValue = Int(currentNumberInLabel)!
 buttonResponseLabel.text = ""
 case "*":
 currentOperation = 3
 storedValue = Int(currentNumberInLabel)!
 buttonResponseLabel.text = ""
 case "/":
 currentOperation = 4
 storedValue = Int(currentNumberInLabel)!
 buttonResponseLabel.text = ""
 default:
 if buttonTitle == "="
 {
 var finalValue = 0
 print(currentNumberInLabel)
 let secondStoredValue = Int(currentNumberInLabel)!
 if currentOperation == 1
 {
 finalValue = storedValue + secondStoredValue
 buttonResponseLabel.text = ""
 buttonResponseLabel.text = String(finalValue)
 }
 else if currentOperation == 2
 {
 finalValue = storedValue - secondStoredValue
 buttonResponseLabel.text = ""
 buttonResponseLabel.text = String(finalValue)
 }
 else if currentOperation == 3
 {
 finalValue = storedValue * secondStoredValue
 buttonResponseLabel.text = ""
 buttonResponseLabel.text = String(finalValue)
 }
 else if currentOperation == 4
 {
 finalValue = storedValue / secondStoredValue
 buttonResponseLabel.text = ""
 buttonResponseLabel.text = String(finalValue)
 }
 }
 else
 {
 if userIsCurrentlyTyping
 {
 currentNumberInLabel = buttonResponseLabel.text! + buttonTitle
 buttonResponseLabel.text = currentNumberInLabel
 }
 else
 {
 userIsCurrentlyTyping = true
 buttonResponseLabel.text = ""
 buttonResponseLabel.text = buttonTitle
 }
 }
 print("\(currentOperation) \(buttonTitle)")
 }
 }
}

• John Deters: Efficiency shouldn't be your focus at this point. Aim for correctness first (write unit tests!), then refactor to remove duplication; then consider continued refactoring to improve the design and readability/maintainability. Only then should you worry about efficiency; even then, you only need to improve efficiency if the existing code is causing an actual performance issue.

Answer 1

Try not to repeat yourself and refactor common code to functions. Also you might want to create types (classes, structs, enums) to make things more expressive. Quick example with not so nice new data type (untested):

import UIKit
class ViewController: UIViewController {
 struct Operator {
 var stringRepresentation: String
 var `operator`: (Int, Int) -> Int //quotes required as operator is a keyword
 }
 var operators = ["+":Operator(stringRepresentation:"+", operator:+),
 "-":Operator(stringRepresentation:"-", operator:-),
 "*":Operator(stringRepresentation:"*", operator:*),
 "/":Operator(stringRepresentation:"/", operator:/)]
 @IBOutlet var buttonResponseLabel: UILabel!
 var userIsCurrentlyTyping: Bool = false
 var storedValue: Int = 0;
 var currentOperation: Operator?
 var currentNumberInLabel = ""
 var buttonTitle = ""
 @IBAction func buttonAppend(_ sender: UIButton) {
 buttonTitle = sender.currentTitle!
 if let chosenOperator = operators[buttonTitle] {
 currentOperation = chosenOperator
 storedValue = Int(currentNumberInLabel)!
 buttonResponseLabel.text = ""
 } else if buttonTitle == "=" {
 showResult()
 } else if userIsCurrentlyTyping {
 currentNumberInLabel = buttonResponseLabel.text! + buttonTitle
 buttonResponseLabel.text = currentNumberInLabel
 } else {
 userIsCurrentlyTyping = true
 buttonResponseLabel.text = buttonTitle
 }
 print("\(currentOperation) \(buttonTitle)")
 }
 func showResult() {
 print(currentNumberInLabel)
 guard let currentOperator = currentOperation else {
 print("This should never happen. Fail silently")
 return
 }
 operate(withOperator: currentOperator.operator)
 }
 func operate(withOperator op: (Int, Int) -> Int){
 let secondStoredValue = Int(currentNumberInLabel)!
 let finalValue = op(storedValue, secondStoredValue)
 buttonResponseLabel.text = String(finalValue)
 }
}

A very good walk-through Swift by implementing a calculator can be found here btw. It's from late 2015, so no Swift3. Maybe you find an update.

• bsoleimany: Any way to explain what is going on in this part of the code? var operators = ["+":Operator(stringRepresentation:"+", theOperator:+), "-":Operator(stringRepresentation:"-", theOperator:-), "*":Operator(stringRepresentation:"*", theOperator:*), "/":Operator(stringRepresentation:"/", theOperator:/)]
• shallowThought: This is instantiating a Dictionary or type [String:Operator] with an entry for each possible operator. It is a short for for: var operators = [String:Operator]() operators["+"] = Operator(stringRepresentation:"+", theOperator:+) operators["-"] = Operator(stringRepresentation:"-", theOperator:-) ...

Answer 2

Something that I really enjoy doing in Swift is making use of of enumerations.

You could create a CalculatorOperation enum that extracts some of your code into an type like this:

enum CalculatorOperation {
 case add
 case subtract
 case multiply
 case divide
 init?(from buttonTitle: String) {
 switch buttonTitle {
 case "+": self = .add
 case "-": self = .subtract
 case "*": self = .multiply
 case "/": self = .divide
 default: return nil
 }
 }
 func apply(to left: Int, and right: Int) -> Int {
 switch self {
 case .add:
 return left + right
 case .subtract:
 return left - right
 case .multiply:
 return left * right
 case .divide:
 return left / right
 }
 }
}

Then, your view controller becomes a lot more manageable.

I'd also separate out your @IBActions so that you have one for when people tap on an operation button, and another action specifically for dealing with when people tap the '=' button.

class ViewController: UIViewController {
 @IBOutlet var inputLabel: UILabel!
 var firstNumber: Int = 0
 var currentOperation: CalculatorOperation? = nil
 // connect 1,2,3,4,5,6,7,8,9,0 buttons to this action
 @IBAction func updateNumber(_ numberButton: UIButton) {
 // update the input label with whatever number has been pressed
 let numberTitle = numberButton.currentTitle!
 inputLabel.text = inputLabel.text! + numberTitle
 }
 // connect '+', '-', '*', '/' buttons to this action
 @IBAction func updateOperation(_ operationButton: UIButton) {
 // update the operation based on the key pressed
 let operationTitle = operationButton.currentTitle!
 currentOperation = CalculatorOperation(from: operationTitle)
 // save the number shown in the input as the first number
 let currentinput = inputLabel.text!
 firstNumber = Int(currentinput)!
 // clear out the input label so it's ready for new number to be entered
 inputLabel.text = ""
 }
 // connect '=' button to this action
 @IBAction func performCalculation(_ sender: UIButton) {
 // get the second number from whatever is currently in the input label
 let currentinput = inputLabel.text!
 let secondNumber = Int(currentinput)!
 let finalValue = currentOperation!.apply(to: firstNumber, and: secondNumber)
 // update input label with the calculated value
 inputLabel.text = String(finalValue)
 }
}

Something else I've found really helpful lately is to start in a playground, and make sure that my logic is working before jumping into a an app.

The playgrounds are always re-compiling in the background as you type, so it's a great way to get early feedback on logic errors and spot bugs.

Here's an example where I noticed that the division isn't worked as expected because we're using Int (so should probably cast to a Double)

enter image description here

• Alexander: You should make this enum have a String rawValue, so it gets rid of the need of manually making an initializer that switches on strings.