patternswiftMinor
Clear function for Stanford iOS 8 Calculator application
Viewed 0 times
stanfordiosapplicationfunctionforclearcalculator
Problem
I am following the Stanford iOS 8 lectures and I believe I have successfully added the "Clear" functionality to the application, but I don't know if I am doing it properly.
Controller
Model
```
import Foundation
class CalculatorBrain
{
private enum Op: Printable
{
case Operand(Double)
case UnaryOperation(String, Double -> Double)
case BinaryOperation(String, (Double, Double) -> Double)
var description: String
{
get
{
Controller
import UIKit
class ViewController: UIViewController
{
@IBOutlet weak var display: UILabel!
var userIsInTheMiddleOfTypingANumber = false
var brain = CalculatorBrain()
var displayValue: Double
{
get
{
return NSNumberFormatter().numberFromString(display.text!)!.doubleValue
}
set
{
display.text = "\(newValue)"
userIsInTheMiddleOfTypingANumber = false
}
}
@IBAction func appendDigit(sender: UIButton)
{
let digit = sender.currentTitle!
if userIsInTheMiddleOfTypingANumber
{
display.text = display.text! + digit
}
else
{
display.text = digit
userIsInTheMiddleOfTypingANumber = true
}
}
@IBAction func enter()
{
userIsInTheMiddleOfTypingANumber = false
if let result = brain.pushOperand(displayValue){ displayValue = result }
else {displayValue = 0}
}
@IBAction func operate(sender: UIButton)
{
if userIsInTheMiddleOfTypingANumber{ enter() }
if let operation = sender.currentTitle
{
if let result = brain.performOperation(operation) {displayValue = result}
else {displayValue = 0}
}
}
// Linked to the "C" button in the Storyboard
@IBAction func clear()
{
userIsInTheMiddleOfTypingANumber = false
brain.clear()
displayValue = 0
}Model
```
import Foundation
class CalculatorBrain
{
private enum Op: Printable
{
case Operand(Double)
case UnaryOperation(String, Double -> Double)
case BinaryOperation(String, (Double, Double) -> Double)
var description: String
{
get
{
Solution
func clear()
{
opStack = [Op]()
evaluate()
}I'm just going to review this method for now. You said in comments that most of this code was provided from a lecture and the primary task was implementing this method.
First of all, calling
evaluate() seems particularly unuseful. evaluate() doesn't actually change anything about the state of the operation stack. All we get out of this is the println statement. Given that this is an iOS app, println is going to be just wasted processor time (and should be eliminated from release builds). The only reason to call evaluate is if you're going to do something with its return value.Additionally, rather than creating a new array, why don't we just empty the existing array?
opStack.removeAll()So, our final implementation should either care or not care about what
evaluate() returns.If we choose to care, we should make
evaluate() more like the other methods were we are pushing buttons other than clear:func clear() -> Double? {
opStack.removeAll()
return evaluate()
}In this case,
evaluate() and therefore clear() should always return nil, but it might be useful for unit testing to go ahead and return the value. We can always assert the nil-ness in unit testing.assert(clear() == nil, "clear() must return nil.")If we choose to not care, calling
evaluate is unnecessary:func clear() {
opStack.removeAll()
}ADDENDUM: I'm also going to argue in your view controller that we shouldn't ever need to write the line
displayValue = 0. I know that it happens throughout the provided code, but I don't consider it all that great. In all cases with the if let/else, it would be simply to just write:displayValue = brain.performOperation(operation) ?? 0instead of the big
if let/else which we've smushed down to save lines.In the case of your implementation of the
clear() @IBAction method, it's fine as is if we leave our clear() with no return value. However, if we give it a return value, we should repeat our use of Swift's nil-coalescing operator:displayValue = clear() ?? 0Code Snippets
func clear()
{
opStack = [Op]()
evaluate()
}opStack.removeAll()func clear() -> Double? {
opStack.removeAll()
return evaluate()
}assert(clear() == nil, "clear() must return nil.")func clear() {
opStack.removeAll()
}Context
StackExchange Code Review Q#84866, answer score: 4
Revisions (0)
No revisions yet.