Stanford developing calculator iOS app assignment

Question

I'm currently learning Swift by following the Stanford developing iOS app 2015 lecture series. I have completed assignment 1 but am wondering if there is any improvement can be made to my code.

Also, I have not been able to complete extra credit no. 4, so I would appreciate if someone could offer me a hint.

import UIKit
class ViewController: UIViewController {
 @IBOutlet weak var display: UILabel!
 @IBOutlet weak var history: UILabel!
 var userIsInTheMiddleOfTyping = false
 @IBAction func appendDigit(sender: UIButton) {
 let digit = sender.currentTitle!
 if digit == "." && display.text!.rangeOfString(".") != nil {
 return
 }
 if userIsInTheMiddleOfTyping {
 display.text = display.text! + digit
 } else {
 display.text = digit
 userIsInTheMiddleOfTyping = true
 }
 }
 @IBAction func operate(sender: UIButton) {
 if userIsInTheMiddleOfTyping {
 enter()
 }
 let symbol = sender.currentTitle!
 appendToEntryRecord(symbol)
 appendToEntryRecord(" = ")
 switch symbol {
 case "×ばつ": performOperation {0ドル * 1ドル}
 case "÷": performOperation { 1ドル / 0ドル}
 case "+": performOperation {0ドル + 1ドル}
 case "−": performOperation {1ドル - 0ドル}
 case "√": performOperation {sqrt(0ドル)}
 case "sin": performOperation {sin(0ドル)}
 case "cos": performOperation {cos(0ドル)}
 case "tan": performOperation {tan(0ドル)}
 case "π": performOperation {M_PI}
 default: break
 }
 }
 func performOperation(operation: (Double, Double) -> Double){
 if operandStack.count >= 2 {
 displayValue = operation(operandStack.removeLast(), operandStack.removeLast())
 enter()
 }
 }
 @nonobjc
 func performOperation(operation: (Double) -> Double){
 if operandStack.count >= 1{
 displayValue = operation(operandStack.removeLast())
 enter()
 }
 }
 @nonobjc
 func performOperation(operation: () -> Double){
 displayValue = operation()
 enter()
 }
 var operandStack = [Double]()
 @IBAction func enter() {
 userIsInTheMiddleOfTyping = false
 appendToEntryRecord("\(displayValue)")
 operandStack.append(displayValue)
 print("operandStack = \(operandStack)")
 }
 var displayValue: Double {
 get {
 return NSNumberFormatter().numberFromString(display.text!)!.doubleValue
 }
 set {
 display.text = "\(newValue)"
 userIsInTheMiddleOfTyping = false
 }
 }
 func appendToEntryRecord(value: String){
 history.text = history.text! + " " + value
 }
 @IBAction func clear() {
 display.text = "0"
 history.text = "0"
 operandStack = [Double]()
 }
 @IBAction func backspace() {
 if display.text!.characters.count > 0 {
 display.text = String(display.text!.characters.dropLast())
 }
 }
 @IBAction func changeSign() {
 if userIsInTheMiddleOfTyping {
 display.text = "-\(display.text!)"
 } else {
 performOperation {-0ドル}
 }
 }
}

enter image description here enter image description here enter image description here

• nhgrif: There is a lot of code here, and I don't find it particularly interesting in general (I've seen this exercise a million times). My two major comments (not a full review here) would be you're not safely handling optionals anywhere and you should never be calling methods marked as @IBAction in code. If the code run in an @IBAction method needs to also be called from elsewhere, it should be refactored into another method that's not an @IBAction. But you need to drastically rethink the way you're handling optionals.
• Tim Vermeulen: @nhgrif What's wrong with calling an @IBAction manually? I agree that you could put the relevant code in another function, but is there really a point of doing that if you can just call the @IBAction directly?
• nhgrif: Everything is wrong with calling an @IBAction directly. It's a button handle, it should handle the tap of a button and nothing more.

Answer 1

I have never seen the Stanford class or this example before, so I might be mentioning stuff that hasn't been covered yet in the class...

---

Inappropriate use of IBActions:

Much like viewDidLoad() and family, @IBActions are for telling your class that something has happened, not for telling it to do things. As such, you should name such functions to remind yourself of this fact. Also, that's why you don't call them directly in code.

For example, your appendDigit function doesn't actually append a digit. Sometimes it ignores the value (if the "digit" is "." and the text already has a dot in it,) and sometimes it wipes out the text and replaces it with the digit. This method exists to let your class know that one of the number keys was tapped, so a better name would be something like @IBAction func tappedNumberButton(sender: UIButton)

---

All your code is in the view controller, there is no Model:

You are writing a calculator app, but there isn't any expressed concept of a Calculator in your code. I would expect to see some sort of calculator class that deals with numbers and manipulating them. Something that exists independent of the view controller and could be used as a component in other apps that need to handle calculation.

Having a separate Calculator class would also make your code amiable to automated tests.

---

You seem to have two distinct states in your system as evidenced by your userIsInTheMiddleOfTyping variable. It would be interesting if you actually expressed that in two different classes, but that might be overkill for this particular project.

---

Your indentation seems consistent, but your use of carnage return isn't. Sometimes you use two CRs.

• nhgrif: This is a good first answer. Welcome to Code Review.