layout: post
title: “Calculator Walkthrough: Part 3”
description: “Adding the services and user interface, and dealing with disaster”
categories: [“Worked Examples”]
seriesId: “Annotated walkthroughs”
seriesOrder: 3
In this post, I’ll continue developing a simple pocket calculator app.
In the first post, we completed a first draft of the design, using only types (no UML diagrams!).
and in the previous post, we created an initial implementation that exercised the design and revealed a missing requirement.
Now it’s time to build the remaining components and assemble them into a complete application
Creating the services
We have a implementation. But the implementation depends on some services, and we haven’t created the services yet.
In practice though, this bit is very easy and straightforward. The types defined in the domain enforce constraints
such there is really only one way of writing the code.
I’m going to show all the code at once (below), and I’ll add some comments afterwards.
// ================================================
// Implementation of CalculatorConfiguration
// ================================================
module CalculatorConfiguration =
// A record to store configuration options
// (e.g. loaded from a file or environment)
type Configuration = {
decimalSeparator : string
divideByZeroMsg : string
maxDisplayLength: int
}
let loadConfig() = {
decimalSeparator =
System.Globalization.CultureInfo.CurrentCulture.NumberFormat.CurrencyDecimalSeparator
divideByZeroMsg = "ERR-DIV0"
maxDisplayLength = 10
}
// ================================================
// Implementation of CalculatorServices
// ================================================
module CalculatorServices =
open CalculatorDomain
open CalculatorConfiguration
let updateDisplayFromDigit (config:Configuration) :UpdateDisplayFromDigit =
fun (digit, display) ->
// determine what character should be appended to the display
let appendCh=
match digit with
| Zero ->
// only allow one 0 at start of display
if display="0" then "" else "0"
| One -> "1"
| Two -> "2"
| Three-> "3"
| Four -> "4"
| Five -> "5"
| Six-> "6"
| Seven-> "7"
| Eight-> "8"
| Nine-> "9"
| DecimalSeparator ->
if display="" then
// handle empty display with special case
"0" + config.decimalSeparator
else if display.Contains(config.decimalSeparator) then
// don't allow two decimal separators
""
else
config.decimalSeparator
// ignore new input if there are too many digits
if (display.Length > config.maxDisplayLength) then
display // ignore new input
else
// append the new char
display + appendCh
let getDisplayNumber :GetDisplayNumber = fun display ->
match System.Double.TryParse display with
| true, d -> Some d
| false, _ -> None
let setDisplayNumber :SetDisplayNumber = fun f ->
sprintf "%g" f
let setDisplayError divideByZeroMsg :SetDisplayError = fun f ->
match f with
| DivideByZero -> divideByZeroMsg
let doMathOperation :DoMathOperation = fun (op,f1,f2) ->
match op with
| Add -> Success (f1 + f2)
| Subtract -> Success (f1 - f2)
| Multiply -> Success (f1 * f2)
| Divide ->
try
Success (f1 / f2)
with
| :? System.DivideByZeroException ->
Failure DivideByZero
let initState :InitState = fun () ->
{
display=""
pendingOp = None
}
let createServices (config:Configuration) = {
updateDisplayFromDigit = updateDisplayFromDigit config
doMathOperation = doMathOperation
getDisplayNumber = getDisplayNumber
setDisplayNumber = setDisplayNumber
setDisplayError = setDisplayError (config.divideByZeroMsg)
initState = initState
}
Some comments:
- I have created a configuration record that stores properties that are used to parameterize the services, such as the decimal separator.
- The configuration record is passed into the
createServices
function, which in turn passes the configuration on those services that need it. - All the functions use the same approach of returning one of the types defined in the design, such as
UpdateDisplayFromDigit
orDoMathOperation
. - There are only a few tricky edge cases, such as trapping exceptions in division, or preventing more than one decimal separator being appended.
Creating the user interface
For the user interface, I’m going to use WinForms rather than WPF or a web-based approach. It’s simple and should work on Mono/Xamarin as well as Windows.
And it should be easy to port to other UI frameworks as well.
As is typical with UI development I spent more time on this than on any other part of the process!
I’m going to spare you all the painful iterations and just go directly to the final version.
I won’t show all the code, as it is about 200 lines (and you can see it in the gist), but here are some highlights:
module CalculatorUI =
open CalculatorDomain
type CalculatorForm(initState:InitState, calculate:Calculate) as this =
inherit Form()
// initialization before constructor
let mutable state = initState()
let mutable setDisplayedText =
fun text -> () // do nothing
The CalculatorForm
is a subclass of Form
, as usual.
There are two parameters for its constructor.
One is initState
, the function that creates an empty state, and calculate
, the function that transforms the state based on the input.
In other words, I’m using standard constructor based dependency injection here.
There are two mutable fields (shock horror!).
One is the state itself. Obviously, it will be modified after each button is pressed.
The second is a function called setDisplayedText
. What’s that all about?
Well, after the state has changed, we need to refresh the control (a Label) that displays the text.
The standard way to do it is to make the label control a field in the form, like this:
type CalculatorForm(initState:InitState, calculate:Calculate) as this =
inherit Form()
let displayControl :Label = null
and then set it to an actual control value when the form has been initialized:
member this.CreateDisplayLabel() =
let display = new Label(Text="",Size=displaySize,Location=getPos(0,0))
display.TextAlign <- ContentAlignment.MiddleRight
display.BackColor <- Color.White
this.Controls.Add(display)
// traditional style - set the field when the form has been initialized
displayControl <- display
But this has the problem that you might accidentally try to access the label control before it is initialized, causing a NRE.
Also, I’d prefer to focus on the desired behavior, rather than having a “global” field that can be accessed by anyone anywhere.
By using a function, we (a) encapsulate the access to the real control and (b) avoid any possibility of a null reference.
The mutable function starts off with a safe default implementation (fun text -> ()
),
and is then changed to a new implementation when the label control is created:
member this.CreateDisplayLabel() =
let display = new Label(Text="",Size=displaySize,Location=getPos(0,0))
this.Controls.Add(display)
// update the function that sets the text
setDisplayedText <-
(fun text -> display.Text <- text)
Creating the buttons
The buttons are laid out in a grid, and so I create a helper function getPos(row,col)
that gets the physical position from a logical (row,col) on the grid.
Here’s an example of creating the buttons:
member this.CreateButtons() =
let sevenButton = new Button(Text="7",Size=buttonSize,Location=getPos(1,0),BackColor=DigitButtonColor)
sevenButton |> addDigitButton Seven
let eightButton = new Button(Text="8",Size=buttonSize,Location=getPos(1,1),BackColor=DigitButtonColor)
eightButton |> addDigitButton Eight
let nineButton = new Button(Text="9",Size=buttonSize,Location=getPos(1,2),BackColor=DigitButtonColor)
nineButton |> addDigitButton Nine
let clearButton = new Button(Text="C",Size=buttonSize,Location=getPos(1,3),BackColor=DangerButtonColor)
clearButton |> addActionButton Clear
let addButton = new Button(Text="+",Size=doubleHeightSize,Location=getPos(1,4),BackColor=OpButtonColor)
addButton |> addOpButton Add
And since all the digit buttons have the same behavior, as do all the math op buttons, I just created some helpers that set the event handler in a generic way:
let addDigitButton digit (button:Button) =
button.Click.AddHandler(EventHandler(fun _ _ -> handleDigit digit))
this.Controls.Add(button)
let addOpButton op (button:Button) =
button.Click.AddHandler(EventHandler(fun _ _ -> handleOp op))
this.Controls.Add(button)
I also added some keyboard support:
member this.KeyPressHandler(e:KeyPressEventArgs) =
match e.KeyChar with
| '0' -> handleDigit Zero
| '1' -> handleDigit One
| '2' -> handleDigit Two
| '.' | ',' -> handleDigit DecimalSeparator
| '+' -> handleOp Add
// etc
Button clicks and keyboard presses are eventually routed into the key function handleInput
, which does the calculation.
let handleInput input =
let newState = calculate(input,state)
state <- newState
setDisplayedText state.display
let handleDigit digit =
Digit digit |> handleInput
let handleOp op =
Op op |> handleInput
As you can see, the implementation of handleInput
is trivial.
It calls the calculation function that was injected, sets the mutable state to the result, and then updates the display.
So there you have it — a complete calculator!
Let’s try it now — get the code from this gist and try running it as a F# script.
Disaster strikes!
Let’s start with a simple test. Try entering 1
Add
2
Equals
. What would you expect?
I don’t know about you, but what I wouldn’t expect is that the calculator display shows 12
!
What’s going on? Some quick experimenting shows that I have forgotten something really important —
when an Add
or Equals
operation happens, any subsequent digits should not be added to the current buffer, but instead start a new one.
Oh no! We’ve got a showstopper bug!
Remind me again, what idiot said “if it compiles, it probably works”.*
* Actually, that idiot would be me (among many others).
So what went wrong then?
Well the code did compile, but it didn’t work as expected, not because the code was buggy, but because my design was flawed.
In other words, the use of the types from the type-first design process means that I do have high confidence that the code I wrote is a correct implementation of the design.
But if the requirements and design are wrong, all the correct code in the world can’t fix that.
We’ll revisit the requirements in the next post, but meanwhile, is there a patch we can make that will fix the problem?
Fixing the bug
Let’s think of the circumstances when we start a new set of digits, vs. when we just append to the existing ones.
As we noted above, a math operation or Equals
will force the reset.
So why not set a flag when those operations happen? If the flag is set, then start a new display buffer,
and after that, unset the flag so that characters are appended as before.
What changes do we need to make to the code?
First, we need to store the flag somewhere. We’ll store it in the CalculatorState
of course!
type CalculatorState = {
display: CalculatorDisplay
pendingOp: (CalculatorMathOp * Number) option
allowAppend: bool
}
(This might seem like a good solution for now, but using flags like this is really a design smell.
In the next post, I’ll use a different approach which doesn’t involve flags)
Fixing the implementation
With this change made, compiling the CalculatorImplementation
code now breaks everywhere a new state is created.
Actually, that’s what I like about using F# — something like adding a new field to a record is a breaking change, rather than
something that can be overlooked by mistake.
We’ll make the following tweaks to the code:
- For
updateDisplayFromDigit
, we return a new state withallowAppend
set to true. - For
updateDisplayFromPendingOp
andaddPendingMathOp
, we return a new state withallowAppend
set to false.
Fixing the services
Most of the services are fine. The only service that is broken now is initState
, which just needs to be tweaked to have allowAppend
be true when starting.
let initState :InitState = fun () ->
{
display=""
pendingOp = None
allowAppend = true
}
Fixing the user interface
The CalculatorForm
class continues to work with no changes.
But this change does raise the question of how much the CalculatorForm
should know about the internals of the CalculatorDisplay
type.
Should CalculatorDisplay
be transparent, in which case the form might break every time we change the internals?
Or should CalculatorDisplay
be an opaque type, in which case we will need to add another “service” that extracts the buffer from the CalculatorDisplay
type so that the form
can display it?
For now, I’m happy to tweak the form if there are changes. But in a bigger or more long-term project,
when we are trying to reduce dependencies, then yes, I would make the domain types opaque as much as possible to reduce the fragility of the design.
Testing the patched version
Let’s try out the patched version now (you can get the code for the patched version from this gist).
Does it work now?
Yes. Entering 1
Add
2
Equals
results in 3
, as expected.
So that fixes the major bug. Phew.
But if you keep playing around with this implementation, you will encounter other bugs undocumented features too.
For example:
1.0 / 0.0
displaysInfinity
. What happened to our divide by zero error?- You get strange behaviors if you enter operations in unusual orders. For example, entering
2 + + -
shows8
on the display!
So obviously, this code is not yet fit for purpose.
What about Test-Driven Development?
At this point, you might be saying to yourself: “if only he had used TDD this wouldn’t have happened”.
It’s true — I wrote all this code, and yet I didn’t even bother to write a test that checked whether you could add two numbers properly!
If I had started out by writing tests, and letting that drive the design, then surely I wouldn’t have run into this problem.
Well in this particular example, yes, I would probably would have caught the problem immediately.
In a TDD approach, checking that 1 + 2 = 3
would have been one of the first tests I wrote!
But on the other hand, for obvious flaws like this, any interactive testing will reveal the issue too.
To my mind, the advantages of test-driven development are that:
- it drives the design of the code, not just the implementation.
- it provides guarantees that code stays correct during refactoring.
So the real question is, would test-driven development help us find missing requirements or subtle edge cases?
Not necessarily. Test-driven development will only be effective if we can think of every possible case that could happen in the first place.
In that sense, TDD would not make up for a lack of imagination!
And if do have good requirements, then hopefully we can design the types to make illegal states unrepresentable
and then we won’t need the tests to provide correctness guarantees.
Now I’m not saying that I am against automated testing. In fact, I do use it all the time to verify certain requirements, and especially for integration and testing in the large.
So, for example, here is how I might test this code:
module CalculatorTests =
open CalculatorDomain
open System
let config = CalculatorConfiguration.loadConfig()
let services = CalculatorServices.createServices config
let calculate = CalculatorImplementation.createCalculate services
let emptyState = services.initState()
/// Given a sequence of inputs, start with the empty state
/// and apply each input in turn. The final state is returned
let processInputs inputs =
// helper for fold
let folder state input =
calculate(input,state)
inputs
|> List.fold folder emptyState
/// Check that the state contains the expected display value
let assertResult testLabel expected state =
let actual = state.display
if (expected <> actual) then
printfn "Test %s failed: expected=%s actual=%s" testLabel expected actual
else
printfn "Test %s passed" testLabel
let ``when I input 1 + 2, I expect 3``() =
[Digit One; Op Add; Digit Two; Action Equals]
|> processInputs
|> assertResult "1+2=3" "3"
let ``when I input 1 + 2 + 3, I expect 6``() =
[Digit One; Op Add; Digit Two; Op Add; Digit Three; Action Equals]
|> processInputs
|> assertResult "1+2+3=6" "6"
// run tests
do
``when I input 1 + 2, I expect 3``()
``when I input 1 + 2 + 3, I expect 6``()
And of course, this would be easily adapted to using NUnit or similar.
How can I develop a better design?
I messed up! As I said earlier, the implementation itself was not the problem. I think the type-first design process worked.
The real problem was that I was too hasty and just dived into the design without really understanding the requirements.
How can I prevent this from happening again next time?
One obvious solution would be to switch to a proper TDD approach.
But I’m going to be a bit stubborn, and see if I can stay with a type-first design!
In the next post, I will stop being so ad-hoc and over-confident,
and instead use a process that is more thorough and much more likely to prevent these kinds of errors at the design stage.
The code for this post is available on GitHub in this gist (unpatched)
and this gist (patched).