HiveBrain v1.2.0
Get Started
← Back to all entries
patternswiftMinor

Handling calculator keypresses

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
handlingcalculatorkeypresses

Problem

I've added decimal (floating point) functionality to my calculator implementation; it's working but I'm having trouble making it more concise and less ugly.

I'm trying to use the same method to handle both number keys and the decimal point, and whichever way I look at it, I either have to check for the button being pressed and then whether the user is just starting a new entry, or the other way around. I've even drawn a flowchart to see how could this be condensed with nested logical operators (||, &&) but couldn't get it to work.

Obviously I could write a separate method for the decimal key, but then I'd get to repeating the isTyping verification there instead. Is there a simple way to do this or is code repetition inevitable at some point or another?

@IBAction func buttonDigit(sender: UIButton) {
    let digit = sender.currentTitle!

    if isTyping {
        if digit == "." {
            if !hasDecimal {
                display.text = display.text! + digit
                hasDecimal = true
            }
        } else {
            display.text = display.text! + digit
        }
    } else {
        if digit == "." {
            display.text = "0" + digit
        } else {
            display.text = digit
        }
        isTyping = true
    }
}

Solution

I suggest hooking up the . button to a separate IBAction. The decimal point isn't a digit, so the function name is confusing. Furthermore, the code to handle a decimal point has little in common with the code to handle a digit. There's no point in mashing the code together into the same function.

@IBAction func buttonDecimalPoint(sender: UIButton) {
    if !isTyping {
        display.text = "0."
    } else if !hasDecimal {
        display.text = display.text! + "."
    }
    isTyping = true
    hasDecimal = true
}

@IBAction func buttonDigit(sender: UIButton) {
    if !isTyping {
        display.text = ""
    }
    display.text = display.text! + sender.currentTitle!
    isTyping = true
}


Note that in buttonDecimalPoint, I test if !isTyping rather than if isTyping, so that I can write else if instead of nesting the conditionals like this:

@IBAction func buttonDecimalPoint(sender: UIButton) {
    if isTyping {
        if !hasDecimal {
            display.text = display.text! + "."
        }
    } else {
        display.text = "0."
    }
    isTyping = true
    hasDecimal = true
}


Another benefit is that the not-typing state can be thought of as the "initial" state, so it's more fitting that the code to handle it should come first.

Furthermore, I've moved the = true assignments to the end of each function to make the invariants obvious. After pressing ., you know that isTyping and hasDecimal are both true.

Code Snippets

@IBAction func buttonDecimalPoint(sender: UIButton) {
    if !isTyping {
        display.text = "0."
    } else if !hasDecimal {
        display.text = display.text! + "."
    }
    isTyping = true
    hasDecimal = true
}

@IBAction func buttonDigit(sender: UIButton) {
    if !isTyping {
        display.text = ""
    }
    display.text = display.text! + sender.currentTitle!
    isTyping = true
}
@IBAction func buttonDecimalPoint(sender: UIButton) {
    if isTyping {
        if !hasDecimal {
            display.text = display.text! + "."
        }
    } else {
        display.text = "0."
    }
    isTyping = true
    hasDecimal = true
}

Context

StackExchange Code Review Q#90615, answer score: 4

Revisions (0)

No revisions yet.