Thinking.txt

This document is meant to give insight into my thinking before starting on the problem.

Helpfully, the challenge provides a number of features in the form of stories.

Reading those features, I can see that the vending machine's interface is getting defined. 

INPUT: coins, button presses, data on internal inventory, data on internal change-making abilities
OUTPUT: coins, display, vended items

Interestingly there is no requirement to allow paper bills or credit cards to be used for payment. 

Organizing this project, I can see that there might be a need for supporting services, such as an inventory service to manage the actual inventory of the machine and ...what do I call it? A bank? I'm going to call it a bank, which manages the ability to provide change.

I recall learning a long time ago (high school?) that the coins you put into the machine are not the same coins used to make change, so there would need to be a separate storage area for the bank's change and presumably a controller to let the main system know how much change was still available. The money used to pay for the snacks would go into a coin box as an input only. Perhaps a future feature would be to indicate that the machine can't be used because the coin box is full.

There would need to be a way to keep the entered coins until either a successful selection is made or the coins are returned. Doing some research, there's some fun hardware that determines if a coins is a quarter, nickel, dime, or invalid (penny or fake), so the software will probably need to interface with that hardware through an API. 

-------------------------------------------------------------------------------
Let's start with the first feature, Accept Coins.

After some sketching, I decided to start creating a CoinValidator class. It will interface with the hardware through an API to let it know when a coin has been added and what kind. It will decide if it should keep the coins and if it should reject them. And it should keep a running total of the money used. Internally, however, it should probably keep an array of the coins, so it knows what to release back to the user later. 

Actually, is it necessary? The hardware knows what coins are what. This code should just need to accept that there are real coins out in the real world and that they are being handled.

Except the story's explanation indicates that my code is what needs to determine if something is valid. That is, apparently this code is in the controller and is doing the checks.

Ok, so then the validator needs to be queried: given a coin with such and such characteristics, should I reject it or keep it?

Actually, now I see that there is a validator (reject/keep decision) and an identifier (which coin is it). 

The identifier should be given a weight and a size. Ultimately it would be nice to have these pieces of data provided in a file so it is easy to update the vending machine with new valid coin types, such as when the mint decides to offer special kinds of coins. What if the buffalo nickel is reintroduced? In the meantime, I am going to look up the weights of the coins and hardcode them in. Oh, hey, https://www.usmint.gov/learn/coin-and-medal-programs/coin-specifications is handy.

I realize the size isn't in one dimension. There is the diameter of the coin as well as the thickness of the coin. 

And I know from experience that using ints is better than using floats, even if floats are an intuitive choice. Checking equality of floats is awkward, and so the ints will need to be multiplied in a standard way. So, if a quarter is 5.670 grams, it can be represented as 5670. 

The story mentions only weight and size, although real vending machines also use electromagnets to identify the metal types. While I won't work on it, I can imagine such a feature being needed in the future. So, I'm thinking about the open-closed principle and wondering if there is a way to design my identifier to take a collection of objects that each consider some criteria about the coin in question and then queries each object to determine what they all think it is. So, a WeightIdentifier and a SizeIdentifier today, but tomorrow there might be a MetalContentIdentifier. If they all agree, then it is the coin they all say it is, but if they don't agree, it's an invalid coin. Maybe it is overkill to design it that way now, but it's good to keep in mind, and it should be easy to refactor to such a solution later.

Instead of coin validator, I've decided to call it a coin accepter. 

My next test needed a way to add a coin, and I deciced to create a data structure for it. Which drove the need for a CoinCandidate struct. And like my Cents typedef, I am thinking that it might make sense to create typedefs for the weight, diameter, and thickness. I'll worry about refactoring once this test passes, although I already refactored to move the weight, diameter, and thickness enums into their own header so I can use them directly in this test.

For this test, I'm making it pass in the simplest way possible, as per TDD As If You Mean It. I find it is helpful to test-drive in this way, instead of assuming my higher-level design is accurate. Sometimes, I find that a completely different approach appears and works pretty well if I let the tests drive the design like they are supposed to. 

Ultimately, my coin accepter should use the coin identifier to recognize the different coins. My inclination would be to mock out the identifier, inject it into accepter's constructor, and ensure that each class is tested independently. On the other hand, the behavior of the accepter shouldn't rely on implementation details (which are already tested in its own class). The benefit of the former is that if something is broken, the tests will tell me exactly where it went wrong, whereas the advantage of the latter is that it should result in less brittle code. It feels awkward to have two separate unit tests for two separate classes that say "given these quarter characteristics, ensure that it thinks it is a quarter", but they are operating on two different concepts. One is about the coin itself, and the other is about the result of knowing it is a specific coin. 

Now we're using the identifier, but the values are hardcoded. I can already see replacing my switch statement with a map, and providing constants for the Cents. And eventually I would like to be able to keep a collection of CoinTypes, but they aren't really needed until the story Return Coins. Until then, a running total works just fine.

Interestingly enough, we're now adding the ability to give back a rejected coin, which means we need a place to put it. Currently we're rejecting a single coin at a time, but since I know I need to be able to provide multiple coins at a later time, I should use a collection of returned coins. YAGNI says I shouldn't, but in this case, I'll err on the side of "in software, you usually have none or many, never just one". 

Ok, we have a coin accepter doing what it needs to do to satisfy the story. Now we need the vending machine to display different things based on what coins we have in the machine. To start with, we show the default text. 

Now we need the coin accepter and the display to work together. I put the coins in the accepter, but the display tells me what is inside of it. I could have made separate unit tests for each expect, but putting multiple coins in and seeing them total up seemed like a fine thing for this one test. 

I like the symmetry of returning the default text or returning a calculated result, so amountUI() was created. This function is slightly complicated since we need to show the output as both dollars and cents, while Cents is an int. Should there be more tests driving it? I think so, although I hate having a temporary implementation here that forgets that dollars are needed. So, I updated the test to add up to more than $1, even though it makes the test a big of a bigger task to implement. If I run into trouble, I can always backtrack and start slower and with smaller steps. I've implemented a similar piece of functionality in a game I made in 2013 about selling candy in which I needed to represent money, so I have a working reference of my own.

I realize that this test might need to change to accomodate the Exact Change Only story later, but as I don't have a way to query if I have enough money to make change, I'll worry about it later.

I'd like to be able to actually interact with this program, so let's create an App to manage input. It would make for a good demo for the implemented feature.

Technically I added VendingMachineApp.h/.cpp when I initially wrote the lines in the test that needed them, but I didn't want to make such tiny commits to illustrate it.

Ah, ha! I've uncovered a defect! Apparently when I have only 5 cents on the machine, it outputs as 50 cents instead. My test for the app is failing because I had a test gap in my display code.

Ok, now we have a test that reproduces the defect. Now to fix it.

Note, I could have commented out the failing app test, but I decided not to as it wasn't adding much noise, and I think it would be satisfying to have both tests pass with the same fix.

Now to run the demo:

$ ./KataForPT-bin 
INSERT COIN
q
0.25
q
0.50
d
0.60
n
0.65
p
0.65
p
0.65
p
0.65
r
Returned: rejected coin, rejected coin, rejected coin
d
0.75

Works great!

I want to take a moment to think about how I could have done better. I allowed a test gap because I didn't recognize that there could be an issue with how the text is displayed. Instead of testing the individual results of putting in a single coin, I tried to allow a single test to handle a bigger responsibility of showing the amount totalling up as well as assume that each coin was showing correctly. I'm not going to beat myself up about it, but it is something that will give me pause the next time I find myself wanting to get by with fewer tests.

-------------------------------------------------------------------------------
On to the next feature. Part of me wants to implement Return Coins next, since it seems relatively simple since I already have part of it implemented with the rejected coins in the return slot. However, a vending machine that doesn't vend isn't very useful, so Select Product should be next if we're focused on providing value as early as possible.

The description complicates the display logic a bit. If you successfully buy something, the machine says THANK YOU, but if you check it immediately again, it goes back to INSERT COIN. If you can't afford the item selected, it should say PRICE: 1.00, for example, then goes back to whatever it would display based on the money selected.

The other interesting part is that the money put into the machine disappears once a succesful transaction is made. Up until now, the machine kept the money indefinitely, so the total kept increasing so long as you kept putting coins in. As this story doesn't address providing change, adding more money than needed apparently results in the machine eating the change.

But besides how the display and money works, I also need to handle product selection in the first place. I see that there is a button tied to a product, and the product has a price and a description. 

I can see that the display is going to need not only the coin accepter but also access to something that understands those buttons and vending products. Oh, and someone will need to determine if the customer has put in enough money.

Now, should the ProductSelectionService act in a similar way to the coin accepter in that it controls both selection and retrieval? I think for now it makes sense. Perhaps later we might want to have a separate object manage just the inventory. In the meantime, I think the service should be pretty ignorant about money. It allows for the selection and dispensing of items, and that's it. 

Now that I have the select cola test passing, I think I'll add an assertion that there is no dispensed item to start. It will allow for another value to be returned, and I think it will be useful to show that something has changed when you select a product.

Adding the chips and candy products is pretty straightforward. All the infrastructure is already there.

Perhaps it would be good to get back to an empty dispenser.

Now that we have a simple selection service, we need to create something that validates a customer's selection. It should process a selection by checking with the coin accepter to find out the customer has entered enough money. If so, then relay the selection to the selection service. If not, reject the selection. It's a nice facade that provides a simple interface and uses the coin accepter and our new selection service together.

Now, obviously the selection validator has no idea what the status of the money is. That's where the coin accepter would come in, and we'll add it in the next test and so then need to inject it into the validator. And since we'll be passing through the selection to the selection service, we'll inject it as well. It's kind of a bigger jump to write this test, but I'm feeling confident. 

I also notice that adding a coin candidate happens often enough that I want to extract the helpers I created in VendingMachineDisplayFixture so more tests can use them.

Oh, yeah. There is no current functionality to reset the coin accepter. We'll need to change that, as this story asks that we need to be able to have the display say INSERT COIN again with the current amount reset to $0.00. I was originally going to create a reset() function, but I didn't like the name. On top of it, knowing that eventually we need to provide change, I think it makes sense to provide an argument that indicates how much money to keep, with the idea that the coin return will have the remainder returned.

Now we can finish the validator test. I wonder if I'm giving the validator too much responsibility. Should it be in charge of ensuring that purchaseWith() is called only when there is enough money to cover the argument passed in? It might make sense since we're already checking that we have enough money in the first place, but maybe there is some feature envy going on here. The fact that it is getting the total from the accepter, then telling the accepter what amount we're using to purchase a product (with the implication that we know that we can do so) makes me wonder if the logic to determine it should be separate. But then again, what else is the validator supposed to do? 

Well, I'll think about it. In the meantime, I want to refactor the coin adding helpers out, and I also think I should shore up the previous validator test to ensure that no cola has been dispensed.

Oh, and apparently I forgot to refactor the duplicate code out of the two validator tests.

Now to add tests to buy chips and candy for a well-balanced meal.

I like this refactoring. I went from a switch statement to a map and a common chunk of code to handle both cases (and future cases).

Making the candy purchase test pass is a one-line change. Nice!

Now, I should have a test that covers the case of adding some money and ensuring that the coins are eaten by the machine and that purchases don't occur. I expect this one to pass, and it does. It didn't drive the code to change, but it does offer a cheap way to ensure that the validator is working in this case, so I add it.

Ok, so now we have a selector and a validator. Now we need to add some functionality to display messages such as THANK YOU and PRICE: x.xx. The way the display currently works is that it queries the coin accepter to find out if it has any money in it. Unfortunately, the validator does not provide access to a piece of state in a similar way, and I don't think it makes sense for the display to know about individual products to select. So let's change the interface so that selecting a product no longer returns a value, and then we'll provide a separate getter for that value.

I don't like that the validator says you don't have enough money by default. I'd like to have a default state.

Now the display can pay attention to the validator. Actually, no, we need a way for the display to know how much money is needed. It's related to select a product without enough money, so I'll just add an expectation in an existing test. I will also add more tests to cover the other products.

I'm not liking how hardcoded the products and their pricing are. What if we want to remove a product or add another one? But we can worry about it in another kata.

Part of me wants to reset the value of money required, but (1) what's a good default value? 0 doesn't really make sense. And (2), it will be set correctly if the response is NOT_ENOUGH_MONEY_RESPONSE, and otherwise should not be given attention. I suppose a way to ensure it would be to throw an exception if the response type is not correct when calling it. Or we could just say "Don't do that." As it's not meant to be a framework for someone else to write their code with, I want to go with the latter, although in real life I can see an issue with someone else needing to maintain this software if I leave the company or otherwise can't work on the code anymore. So let's be robust about it.

NOW the display can pay attention to the validator. Normally I'm not too worried about using too many mocks. I find that it simplifies my unit test fixtures when I'm not trying to rebuild entire segments of the application, and it allows me to focus on just the system under test. However, each of these components is small, and so while adding yet another one (which itself required another one to be injected into it) makes me worry that the test is getting too large, I haven't yet gotten outside of my comfort zone here. I do, however, want to note that I'm starting to get uncomfortable since I had to introduce the selection service, which is completely unneeded in the display test code except that I need one in order to create the selection validator, which I do need in this test code. 

I suppose it makes sense that the app test would need to be updated since the display requires a new member injected into it, but my uncomfortable feeling from before is a little worse now. I'm updating the display, and now unrelated code and tests had to be impacted. 

We need the display to reset. It can't tell you the price forever!  However, I think it makes sense to require that functionality in the app rather than have the display worry about it. It's already doing a lot, and we expect ui(), which I see now is horribly named, to do potentially too much as it is. It currently needs to determine whether to show the price needed or the amount inserted. Is there a way to make the code more readable?

ui() looks way better now, and someone reading it would find it a lot easier to follow. Including me. 

First, let's make sure all the products are covered, and then we'll worry about thanking the customer.

One thing I liked about my refactoring efforts on ui() before is that new code feels compelled to match the environment it's getting put into. I could have added the code inline, but it would have felt out of place, kind of like showing up to a costume party without a costume.

Ok, we have the display updating to tell us the price and thanking us. Now to put it all together into a working app. I think the products can be associated with the kinds of button IDs you would normally see on a vending machine, which makes me think that there should be some kind of explanation to the user of the app. On a separate note, the display's ui() looks way better than the app's run(). 

At this point, I find a surprise, and I realize that the processing of two characters separately means that the display output will be shown immediately after the first button and not just after both buttons are pressed. 

That is, I'm seeing these test results:

Value of: getNextOutputLine()
Expected: is equal to "PRICE: 1.00"
  Actual: "INSERT COIN" (of type std::string)

Value of: getNextOutputLine()
Expected: is equal to "PRICE: 0.50"
  Actual: "PRICE: 1.00" (of type std::string)

Which means the current output from the actual app probably looks weird, too:

$ ./KataForPT-bin 
INSERT COIN
a1
INSERT COIN
PRICE: 1.00

Yep. So, while I could have changed the test to avoid the test failure, I'll need to change how the implementation itself works to make it feel right.

Ick. I'm not happy with how the code is getting ugly, but it is passing the tests. Note that if you press 2 before pressing b...well, it's probably undefined what happens:

$ ./KataForPT-bin 
INSERT COIN
2
PRICE: 0.00

Ok, well, it's surprising and not how it should work anyway. To fix it, it looks like the validator needs to be updated as well, as selecting NO_PRODUCT defaults to saying we don't have enough money.

Pulling out the text rendering code cleans up the run() function, making it easy to see that it is split into process() and render(). I could refactor some more by creating functions for the contents of the if blocks in render(), but I think it's small enough now.

Still, having a flag to determine if I should show output in a function that says it is going to render the output is kind of gross. Or at least unintuitive. Let's get rid of it with a named function that explains why we're not printing more output.

Ok, we're displaying prices for selected products and ensuring that the customer is pressing buttons in the correct order. I want to move on, but what happens when we press the wrong buttons? 

$ ./KataForPT-bin 
INSERT COIN
a3
PRICE: 0.00

getProductChoice only returns NO_PRODUCT when the buttons are both NO_ID. Is there any reason not to return NO_PRODUCT by default if none of the other if statements apply? The tests pass just fine. 

$ ./KataForPT-bin 
INSERT COIN
a3
INSERT COIN

And it works as expected. So, do I add a test to cover this situation? I think so. Invalid button combinations are different from pressing them in the wrong order.

Ok, now that customers can see what the product costs, let's let them buy them!
Hmm. The test passed. And we didn't need to change the implementation to make it work. I'll admit that surprised me. Looking into it, of course it worked. The display is doing all the hard work of figuring out what to do if the product was purchased. The app is just showing the display's output if it isn't waiting for input or showing returned coins.

We show the price of a selected product and we thank the customer for buying it. Now we need the vending machine to reset itself for the next transaction. We'll use '.' to mean "idle", but any non-processed character will do.

I refactored the test by extracting out the setup code to purchase a cola. I decided to keep the previous test as is, although now the duplication is still there. Perhaps I can separate the setup code better.

It should also reset after the price is displayed, not just when we successfully buy something. 

And we should make sure that it also shows the amount you've already inserted. I expect this test to pass, and it does. Even though it doesn't drive new code, I think it is useful to characterize how this project works. 

And this story is finally finished:

$ ./KataForPT-bin 
INSERT COIN
q
0.25
q
0.50
a1
PRICE: 1.00
.
0.50
.
0.50
q
0.75
q
1.00
a1
THANK YOU
.
INSERT COIN

-------------------------------------------------------------------------------
Next up, Make Change. 

Basically, the code assumes that the customer is putting in the exact amount needed. I now need to allow the customer to insert more money and still be able to purchase an item without the vending machine eating all of the money. 

Currently if I try to purchase a product with too much money in the machine, it tells me the price and doesn't allow the transaction to occur:

qqqqq
1.25
a1
PRICE: 1.00
.
1.25

So, let's change it. I want to be able to (1) buy the product even if I have too much money in the machine and (2) return the remaining amount in denominations that make sense.

This test gets me most of the way there, but the CoinAccepter needs a new way to add returned coins, so maybe I should comment out the new validator test and start on the accepter.

Now that we can return a single coin for either 25, 10, and 5 cents in change, let's return multiple coins for larger amounts. Rather than make a single test, I want to ensure multiple quarters and dimes can be returned. A little thinking makes me realize that I will never want to return multiple nickels (assuming I have enough coins to make change) as I could always return a dime for any two nickels. 

Now we add a test to return multiple denominations, and I expect it to pass just fine. It does. 

So, back to the validator test. 

It occurs to me that the accepter's purchaseWith() could make change. In fact, I setup this function with this idea in mind, and I apparently forgot. So, makeChange() should really be a private helper function.

Currently change is displayed in the app when you check the coin return by pressing 'r'. It's hardcoded to always say it finds rejected coins:

$ ./KataForPT-bin 
INSERT COIN
qqqqqa1
0.25
0.50
0.75
1.00
1.25
THANK YOU
r
Returned: rejected coin

It should instead indicate if it can identify the coins.

And now: 

$ ./KataForPT-bin 
INSERT COIN
qqqb2r
0.25
0.50
0.75
THANK YOU
Returned: quarter

Beautiful! 

-------------------------------------------------------------------------------

For our next story, let's Return Coins. 

A lot of the infrastructure is in place, and so now we'll need to change how we keep track of the customer's money. The coin accepter keeps a running total, but now I want it to keep track of each coin inserted. 

Our test will add a bunch of coins, then expect to get them back instead of getting quarters back. This behavior is different from when change is being made.

Now we need a way to request our money back in the app. When we press the button to return our money, we should see it revert back to having no money and so should say INSERT COIN. Then we can check the return slot and see our money.

And the end result:

$ ./KataForPT-bin 
INSERT COIN
qqqdddnn
0.25
0.50
0.75
0.85
0.95
1.05
1.10
1.15
R
INSERT COIN
r
Returned: quarter, quarter, quarter, dime, dime, dime, nickel, nickel

Perfect.