implement vending application

[Tony Docherty]

Very informative and interesting Junilu, well done and have a cow.

[Ishan Pandya]

A great thread to read. Thank you Junilu for spending time to share the knowledge of TDD.
I followed the thread from the starting and have coded for VendingMachine and CustomerPanel.
I will post some of my code here, For some feedback from Junilu or anyone else.

       @Test  
	public void	hasCoinsDeposited_should_be_true_after_deposit(){
		deposit_one_dollar_in_coins();
		assertThat(testSubject.hasCoinsDeposited(),is(true));
	}
	
	@Test  
	public void hasCoinsDeposited_should_be_true_when_change_is_due() throws Exception {
		deposit_one_dollar_in_coins();
		dispenseProduct(0.75);
		assertThat(testSubject.hasCoinsDeposited(),is(true));
	}
	
	@Test  
	public void hasCoinsDeposited_should_be_false_when_no_change_is_due(){
		deposit_one_dollar_in_coins();
		dispenseProduct(1);
		assertThat(testSubject.hasCoinsDeposited(),is(false));
	}
	
	@Test
	public void hasCoinsDeposited_should_be_false_after_change_returned() throws Exception {  
		deposit_one_dollar_in_coins();
		dispenseProduct(0.75);
		testSubject.return_change_due(); 
		assertThat(testSubject.hasCoinsDeposited(),is(false));
	}
	
	private void dispenseProduct(double dollars){
		testSubject.dispene_product_with_specified_price(dollars);
	}
	
	private void deposit_one_dollar_in_coins(){
		int coin=1;
		testSubject.deposit(coin);
	}

public class CustomerPanel {
	List<Double> coins; //took a list to check which coins were inserted into the vending machine.
	double totalAmountInserted=0.0;
	
	public CustomerPanel(){
		super();
		coins=new ArrayList<Double>();
	}
	
	public void deposit(double coin) {
		coins.add(coin);
		for(Double dollars : coins){
			totalAmountInserted+=dollars;
		}
	}

	public boolean hasCoinsDeposited() {
		return totalAmountInserted!=0;
	}
	
	public void return_change_due() {
		totalAmountInserted=0;
	}

	public void dispene_product_with_specified_price(double dollars) {
		totalAmountInserted-=dollars;
	}
}

I understand that there is no documentation done on any of the method.

Great topic by s ravi chandran.

[Junilu Lacar]

Sorry I haven't been able to respond sooner but I'm a bit swamped at work, having been away on vacation for a few weeks. I appreciate the kind comments from everyone who has put in the time to follow this discussion and will get back to you with some feedback as soon as I get caught up with work in my day job.
Regards to all and I hope you have had a great start to the New Year. From Columbus OH, home of The Ohio State Buckeyes, champions of the first ever College Football Playoff, GO BUCKS!!!

[Junilu Lacar]

Some feedback:

...

public class CustomerPanelTest {

	private CustomerPanel panelInterface;
	
	@Test
	public void initial_amount_should_be_zero()
	{
		int addCoin = 0;
		int totalValueDeposited = panelInterface.addAmount(addCoin);
		assertThat(totalValueDeposited, is(0));
	}
	
        ...
}

First of all, I think you are continuing to show progress. Good job.

initial_amount_should_be_zero() - The name is good: it's to the point and says exactly what the intent of the test is. The test code can still be improved and the way you've written it should have evoked these questions:

1. Why would I have to add a 0 value coin to get the initial amount? Should I have another way of determining how much money has been deposited so far instead? Is this code telling me that I need to add a getter method for amountDeposited?

2. How should this method behave when an invalid coin value is entered? Should I even account for this possibility right now?

When I write a test, I read it back out loud. If I'm working in a group or pair, someone will read it out loud for the others. This helps us "listen" to what the code is saying and see if it makes sense. But here's the trick: sometimes it's better not to read the code verbatim but "translate" it a bit. Here's how I would do that for this test as you have written it:

I have a zero-value coin and I have a new CustomerPanel. I deposit my zero-value coin and I get back the total amount deposited so far, which should be zero.

When you read this out loud, it sounds a little silly and should make you ask the questions above.

Compare that with this:

I have a new CustomerPanel and I haven't deposited any money yet. The CustomerPanel should tell me that the total amount deposited so far is zero.

This sounds more sensible and here's what the test code for it would look like:

    @Test
    public void initial_amount_should_be_zero() {
        CustomerPanel newPanel = new CustomerPanel();
        assertThat(newPanel.getAmount(), is(0));
    }

Line 3, the line that declares local variable newPanel, might represent some duplication with code in the @Before setup method but in this case, I can live with it because I think it makes this particular test code more cohesive by having a local instance of CustomerPanel. We could have also just used the instance that was created in the setup method but that would require a reader to bounce around the code a little. Either approach is fine but I personally prefer the local variable in this case because there's no other code that sets up the context for the test.

Note also that in the second "translation" I say "and I haven't deposited any money yet". There is no actual test code that corresponds to this so it's really an interpretation on my part as I read the code and see that there is no statement where money is actually deposited. That is, the lack of code also means something in this context and the local variable newPanel helps set that context up and make it easier to discern.

Of course, since we're doing TDD, this new version of the test code should fail compilation at first since there should not be a getAmount method yet. Once that's fixed, the test will pass and go Green. That should give you a slight pause because you haven't seen the test fail yet. I would probably just temporarily change the assertion to assertThat(newPanel.getAmount(), is(1)) to see it fail, then put it back to is(0) to get the Green bar back. Another approach would be to intentionally introduce a bug in the CustomerPanel class, perhaps initializing the amount field to a non-zero value in the constructor, just to see the test fail. Then get the Green bar back by removing the intentional bug. Either way, making sure that the test fails first provides you with a sanity check that tells you the test is actually useful for demonstrating some aspect of the behavior of the code under test.

[Junilu Lacar]

Junilu Lacar wrote:
Note also that in the second "translation" I say "and I haven't deposited any money yet". There is no actual test code that corresponds to this so it's really an interpretation on my part as I read the code and see that there is no statement where money is actually deposited. That is, the lack of code also means something in this context and the local variable newPanel helps set that context up and make it easier to discern.

It's kind of difficult to predict whether or not it would actually happen in a live conversation between developers but I could also imagine that if I or someone on my team had said the above, someone could have also offered an alternative: Why not document that in the test name then instead of leaving it up to interpretation, and possibly missing it, by the reader? That is, why don't we refactor the test name to getAmount_should_be_zero_when_new_instance_and_no_coins_deposited or something like that. I would readily agree and accept this refactoring suggestion and would even be amenable to using the instance of CustomerPanel created in the setup method instead of a local variable

    @Test  
    public void getAmount_should_be_zero_when_new_instance_and_no_coins_deposited() {  
        CustomerPanel newPanel = new CustomerPanel();  
        assertThat(newPanel.getAmount(), is(0));
    }

or

    CustomerPanel testPanel;

    @Before
    public void setup() {
         testPanel = new CustomerPanel();
    }

    ...

    @Test 
    public void getAmount_should_be_zero_when_new_instance_and_no_coins_deposited() {  
        assertThat(testPanel.getAmount(), is(0));
    }

Actually, now that I look at both versions, I think I still prefer the former -- I find it tighter and more self-contained. Besides, the local variable name in the first version is more self-documenting and specific to the test scenario, whereas the name testPanel is more general.

[s ravi chandran]

I am searching for this code statement in my project. Current code looks like this.

package com.coderanch.samples.design.displayinterface;

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.*;

import org.junit.Before;
import org.junit.Test;

public class CustomerPanelTest {

	/**  To Be Removed after writing test cases
	 *  1. Deposit coins until you have deposited enough or more than enough to buy at least one product. 
	2. Enter the Tray #, or Slot # if you like that better 
	3. Machine dispenses an item from that Tray or Slot 
	4. Machine gives back change, if necessary 
	 */

	private CustomerPanel panelInterface;

	@Before 
	public void setup() {
		panelInterface = new CustomerPanel();
	}

	@Test
	public void hasCoinsDeposited_should_be_zero_before_deposit ()
	{
		assertThat(panelInterface.getTotalCoinAmount(), is(0));
	}


	@Test
	public void hasCoinsDeposited_should_be_true_after_deposit ()
	{
		int coinAdded = 10;
		int moreCoinsAdded = 20;

		panelInterface.deposit(coinAdded);
		assertThat(panelInterface.hasCoinsDeposited(), is(true));

		panelInterface.deposit(moreCoinsAdded);
		assertThat(panelInterface.hasCoinsDeposited(), is(true));

	}


	@Test
	public void initial_slot_number()  // to be modified
	{
		int selectedSlot = panelInterface.getSelectedSlot();

		assertThat( selectedSlot, is(-1));
	}

	@Test
	public void select_slot_number()  // to be modified
	{
		int slot_number = 2;
		panelInterface.setSelectedSlot(slot_number);
		int selectedSlot = panelInterface.getSelectedSlot();

		assertThat( selectedSlot, is(2));
	}

	@Test
	public void isChangeDue_should_be_true_when_change_is_due() throws Exception {
		deposit_one_dollar_in_coins();
		dispense_75_cent_product();

		assertThat(panelInterface.isChangeDue(), is(true));    
	}


	@Test
	public void isChangeDue_should_be_false_after_change_returned() throws Exception {
		deposit_one_dollar_in_coins();
		dispense_75_cent_product();

		return_change_due();  // how to calculate this? Will need the product cost for calculating change.

		assertThat(panelInterface.isChangeDue(), is(false));    
	}

	@Test
	public void is_sufficient_balance_should_return_false()
	{
		deposit_10_cent_in_coins();
		dispense_25_cent_product();

		boolean is_balance_sufficient = is_deposit_amount_sufficient();
		assertThat(is_balance_sufficient, is(false));
	}

	public void deposit_one_dollar_in_coins()
	{
		int oneDollar = 100; //in cents
		panelInterface.deposit(oneDollar);
	}

	public void dispense_75_cent_product()
	{
		int _75Cents = 75; //in cents
		panelInterface.setProductCost(_75Cents);
	}

	public void return_change_due()
	{
		panelInterface.returnBalanceChange();
	}

	public void dispense_25_cent_product()
	{
		int _25Cents = 25; //in cents
		panelInterface.setProductCost(_25Cents);
	}

	public void deposit_10_cent_in_coins()
	{
		int _10Cents = 10; //in cents
		panelInterface.deposit(_10Cents);
	}

	public boolean is_deposit_amount_sufficient()
	{
		return panelInterface.isSufficientCoinsDeposited();
	}

}

Did I miss something? invalid coin protection needs to be added. I will check that now.

[s ravi chandran]

current customerpanel.java looks like this:

package com.coderanch.samples.design.displayinterface;

public class CustomerPanel {

	private int totalAmountDeposited = 0;
	private int slotPosition = -1;
	private int productCost = 0;

	public void deposit(int coinValue) {

		totalAmountDeposited += coinValue;
	}

	public int getSelectedSlot() {
		return slotPosition;
	}

	public void setSelectedSlot(int slot)
	{
		slotPosition = slot;
	}

	public int getTotalCoinAmount() {
		return totalAmountDeposited;
	}

	public void setTotalCoinAmount(int totalCoinAmount) {
		this.totalAmountDeposited = totalCoinAmount;
	}

	public boolean hasCoinsDeposited() {
		return totalAmountDeposited>0;
	}

	public int getProductCost() {
		return productCost;
	}

	public void setProductCost(int productCost) {
		this.productCost = productCost;
	}

	public boolean isChangeDue()
	{
		int balanceAmountToReturn = totalAmountDeposited - productCost;

		return balanceAmountToReturn>0;
	}

	public int returnBalanceChange() {
		int balanceAmountToReturn = 0;
		
		if(isSufficientCoinsDeposited())
		{
			balanceAmountToReturn = totalAmountDeposited - productCost;  

			totalAmountDeposited =  totalAmountDeposited - balanceAmountToReturn;
		}
		
		return balanceAmountToReturn;
	}

	public boolean isSufficientCoinsDeposited() {

		return totalAmountDeposited>=productCost;
	}

}