implement vending application

[s ravi chandran]

is the role of topUp() to get the quantity to add items to a slot? or is it a replacement for add() ?

[Junilu Lacar]

s ravi chandran wrote:Okay, i need to understand this for now.

===============
| __ __ __ __ | ← one shelf
| __ __ __ __ |
| __ __ __ __ |
| __ __ __ __ |
===============
  ↑
  one tray

Imagine you are looking at the front of the vending machine. One row is a shelf. One element in the row is a tray. Each tray will hold up to 8 pieces of one type of product.

[Junilu Lacar]

s ravi chandran wrote:is the role of topUp() to get the quantity to add items to a slot? or is it a replacement for add() ?

It would seem to me that topUp() is more appropriate for how the vendor would do his/her work of filling up the machine with products, so it would replace add. We are still playing around with the API and trying to find something that makes sense in the grand scheme of things related to the use of a vending machine.

[s ravi chandran]

Junilu Lacar wrote:

s ravi chandran wrote:Okay, i need to understand this for now.

===============
| __ __ __ __ | ← one shelf
| __ __ __ __ |
| __ __ __ __ |
| __ __ __ __ |
===============
  ↑
  one tray

Imagine you are looking at the front of the vending machine. One row is a shelf. One element in the row is a tray. Each tray will hold up to 8 pieces of one type of product.

Got it. So, max product a shelf can handle will be max_slots_per_row * max_products_per_slot. Add() will have to handle these case later on.

[Junilu Lacar]

s ravi chandran wrote:
I added these lines in my code

 public static final int MAX_PRODUCTS_PER_SLOT = 8;

public int topUp(int slot) {
		return MAX_PRODUCTS_PER_SLOT;
	} 

You're breaking TDD rules. You should wait until you have to refactor some code before you introduce a constant like this. Plus, skipped writing a failing test.

[s ravi chandran]

Junilu Lacar wrote:

s ravi chandran wrote:
I added these lines in my code

 public static final int MAX_PRODUCTS_PER_SLOT = 8;

public int topUp(int slot) {
		return MAX_PRODUCTS_PER_SLOT;
	} 

You're breaking TDD rules. You should wait until you have to refactor some code before you introduce a constant like this. Plus, skipped writing a failing test.

ok, sorry. I was seeing this line,

stockCount -= popMachine.topUp(0);

[Junilu Lacar]

s ravi chandran wrote:max product a shelf can handle will be max_slots_per_row * max_products_per_slot.

That's correct but I don't see the number of products per shelf as being relevant to any kind of functionality. That's an incidental statistic. The important number here is the capacity of each tray, i.e. maximum number of items per tray

Also, you have to keep up to date with the terminology. It's tray now, not slot.

[s ravi chandran]

ok. so our topup function should look like this then.

public int topUp(int slot) {
		return 0;
	} 

which will obviously fail when run. then we can add the line

public int topUp(int slot) {
		return 8;
	} 

to make the test pass.

[Junilu Lacar]

Yes! You seem to be getting it now. Now you see the 8 and say, "That's a magic number. Let's extract that to a constant." Now you can go ahead and create your MAX_ITEMS_PER_TRAY constant.

[Junilu Lacar]

Then you write another test, make sure it fails, so you can triangulate the correct implementation of the topUp method.

[s ravi chandran]

Junilu Lacar wrote:Then you write another test, make sure it fails, so you can triangulate the correct implementation of the topUp method.

like this?

@Test  
	public void topUp_should_fill_a_partially_empty_tray() {  
		 VendingMachine popMachine = new VendingMachine(1);  
		 int stockCount = 4;  
	     stockCount -= popMachine.topUp(0); // fill up tray 0  
	     assertThat(stockCount, is(4));  
	}  

[Junilu Lacar]

This technique takes a lot of discipline and dedication to learn to do properly and effectively. You need to read up on refactoring. You need to read up on design principles. Search for Design Principles SOLID, SLAP, DRY. You need to recognize code smells. And you need to question EVERY small design choice you make and look at how it fits into the big picture.

I think I'll leave the rest to you as an exercise. Try to code it all out by yourself and we'll see what you come up with.

[s ravi chandran]

Junilu Lacar wrote:This technique takes a lot of discipline and dedication to learn to do properly and effectively. You need to read up on refactoring. You need to read up on design principles. Search for Design Principles SOLID, SLAP, DRY. You need to recognize code smells. And you need to question EVERY small design choice you make and look at how it fits into the big picture.

I think I'll leave the rest to you as an exercise. Try to code it all out by yourself and we'll see what you come up with.

ok, I will try myself. Thanks..

[Junilu Lacar]

s ravi chandran wrote:

Junilu Lacar wrote:Then you write another test, make sure it fails, so you can triangulate the correct implementation of the topUp method.

like this?

@Test  
	public void topUp_should_fill_a_partially_empty_tray() {  
		 VendingMachine popMachine = new VendingMachine(1);  
		 int stockCount = 4;  
	     stockCount -= popMachine.topUp(0); // fill up tray 0  
	     assertThat(stockCount, is(4));  
	}  

Not quite. That story doesn't make sense. How is tray 0 partially filled up there? At instantiation, all trays are empty. There's nothing in the test that shows you filled it up and then partially emptied it. Something like this makes more sense but it doesn't really test filling up a partially filled tray:

@Test  
public void topUp_cannot_overfill_a_tray() {  
    VendingMachine popMachine = new VendingMachine(1);  
    popMachine.topUp(0); // fill up tray 0  

    int itemsAdded = popMachine.topUp(0); // try to add more product

    assertThat(itemsAdded, is(0));
}  

[Junilu Lacar]

I'm also going to work on it. I have the code up in GitHub:

main: https://github.com/jlacar/ranch-samples/tree/master/samples/src/main/java/com/coderanch/samples/design/vend
test: https://github.com/jlacar/ranch-samples/tree/master/samples/src/test/java/com/coderanch/samples/design/vend

[s ravi chandran]

Junilu Lacar wrote:I'm also going to work on it. I have the code up in GitHub:

main: https://github.com/jlacar/ranch-samples/tree/master/samples/src/main/java/com/coderanch/samples/design/vend
test: https://github.com/jlacar/ranch-samples/tree/master/samples/src/test/java/com/coderanch/samples/design/vend

Ok, meanwhile, i will try to work myself and see if I can make some logical functions for this.

[Junilu Lacar]

I think I've reached a point where VendingMachine has all the functionality I believe it should have. As it turns out, VendingMachine itself only needs a couple of constructors and a few methods: isEmpty, fill, and dispense. As for other functionality like accepting money and making change, it seems like a different class is needed. Let me know when you are ready to continue the discussion.

[s ravi chandran]

Junilu Lacar wrote:I think I've reached a point where VendingMachine has all the functionality I believe it should have. As it turns out, VendingMachine itself only needs a couple of constructors and a few methods: isEmpty, fill, and dispense. As for other functionality like accepting money and making change, it seems like a different class is needed. Let me know when you are ready to continue the discussion.

ok. also, I am trying a different approach from the topUp() function. Will discuss that too once I reach some checkpoint .

[s ravi chandran]

I had added these two methods

	public int topUp(int slot, int stockCount) {
		int stockAdded = stockCount - MAX_ITEMS_PER_TRAY ;
		slots[slot] = MAX_ITEMS_PER_TRAY;
		return stockAdded;
	}
	
	public int topDown(int slot)
	{
		slots[slot] = slots[slot] -1;
	     return slots[slot];	
	}

and it failed at

topUp_should_fill_a_partially_empty_tray()

. So, this won't work. I did take the product add logic from your method though. this line:

slots[slot] = MAX_ITEMS_PER_TRAY;

what I was thinking was to give an option to the vendor to add any number of product to the slot. for which we would definitely have to add a condition to check product count exceeding max value. but i guess, your way is more precise, end target for each add or topUp is to fill the slot to max.

[s ravi chandran]

I have integrated the changes in my code.

[Junilu Lacar]

s ravi chandran wrote:I had added these two methods
public int topUp(int slot, int stockCount) {... public int topDown(int slot) ...

Well, the names you chose there are not very appropriate. The name topUp is an idiomatic expression and it's not the opposite of "top Down", which has an entirely different meaning from what you're using it for. I was afraid this might happen so I changed the name to fill instead. The dispense method was named in anticipation for the need to take away one product when a customer bought something. Most vending machines will dispense only one item at a time since that's normally what people want to do anyway. If a customer wants to buy multiple things, they just go through the buying process multiple times. We can improve on that later, if needed, but for now, I choose to cater to the 90% case of buying only one product at a time.

Also, the parameter name stockCount is a poor one. I used it before to indicate how many items a vendor might have in the stock crate from which he replenishes the vending machine.

Think about it: if you're reading an article that uses the phrase "top down" to mean "take away from" would you think that article was well written? Probably not, right? I have never heard of this meaning or usage for "top down" nor could I find it in any online dictionary. It's not a good name.

Instead of stockCount, a name like quantity makes more sense here. That is, when I read the code fill(tray, quantity), I translate that to a mini conversation that goes like this: "Hey, fill up a tray" ... "Which tray and by how much?" ... "This tray, and by this quantity." It sounds a little weird when you answer the question with "This tray, and by this stockCount." Also, the JavaDocs will read better:

/**
 * Replenish the given tray with the specified number of products.
 *
 * @param tray the tray to replenish
 * @param quantity the number of products to add
 * ...

versus

/**
 * Replenish the given tray with the specified number of products.
 *
 * @param tray the tray to replenish
 * @param stockCount the number of products to add
 * ...

You should be very particular about the names you choose and the meanings that they give to the program. If the names and the intent and/or behavior invoked don't match, it makes understanding the program all the more difficult. Robert "Uncle Bob" Martin devotes an entire chapter in his book "Clean Code" to discussing how to choose good names in your program and the consequences of choosing bad names.

Here's a tip to help you choose better names: Read your code out loud.

When your program is not very clear or when the names you have chosen are not very good, you will hear yourself saying strange and incoherent things when you read the code out loud. If you read poorly written code out loud to other people, they will have a hard time trying to understand what you're saying. Your code needs to make sense when you read it out loud. That's how you write maintainable and understandable programs.

[s ravi chandran]

i had not refractored it. Copied it from previous methods which i had commented.. :-)

[Winston Gutkowski]

s ravi chandran wrote:i had not refractored it. Copied it from previous methods which i had commented.. :-)

My advice: Never comment out code.

If you think you might need it later, copy the .java file to an archive directory as an "old version" and then remove it. If you're using a revision control system like CVS or SubVersion, then it will be done for you automatically; but don't keep it "hanging around" just because you think you might need it later.

Source files (and indeed, documentation) should only include "what IS"; not what "used to be", or "what I think I might possibly need again".

Winston

[s ravi chandran]

okay. will follow that from now on..

[Junilu Lacar]

It seems you have convinced me that there is actually a scenario where a vendor will want to partially replenish a tray. This is the case when they don't have enough stock to fill it completely. Say the vendor has several machines and he runs low on one product. When he gets to the last machine, he may still want to load the remaining product into a tray, even if the tray doesn't get filled up. The story would be: "As a vendor, I want to load a specific quantity of product into the machine in case I'm low on stock and don't have enough to completely fill a tray."

The other story, where you just fill a tray to capacity would be stated as: "As a vendor, I want to know how much product I added to completely fill up a tray so I don't have to keep counting; I can just load up product and have the machine tell me how many I added."

My thinking here is that as a vendor, I'd like to have ways to check and countercheck what I've done while making it easy and quick for me to get my work done of replenishing a machine with more product. First, I'd print a report of the current quantities in the machine. Then I'd just fill trays back up to capacity, letting the machine tell me how many I added, which is why the main version of fill returns that count. In case I run low on product, I can specify exactly how many I added to the last tray when I don't have enough to fill it up completely. Then I can check the count of how many items added to the machine and compare that to my before/after counts in my stock crate. If there are discrepancies, then I can go back to the "preloading" report that I printed out and see where I messed up my counts.

[s ravi chandran]

i had added a logic like this before

int currentProductQuantity = slots[slot];
int totalQuantity = newQuantity + currentProductQuantity;
int quantityAdded = MAX_ITEMS_PER_TRAY - newQuantity;

 if( totalQuantity <= MAX_ITEMS_PER_TRAY)
 {
   slots[slot] = totalQuantity;

}

return quantityAdded;

[Junilu Lacar]

That's another thing about writing programs. I often see developers come up with use cases that, while they are plausible with a stretch of the imagination, they are highly improbable. You have to put yourself in the shoes of the person using your program. Better yet, you should have a conversation with someone who will use your program and have them tell you the stories of how they would do their work. Another good way to find requirements is to observe your customer while they are doing their work and using your program. This way, you will often see them do things that you didn't expect when you were coming up with your latest and greatest creation.

[s ravi chandran]

Yes, I guess, that will come with more understanding of application. Well, I did remove the logic later, as it was failing the partial fill case.

[Junilu Lacar]

s ravi chandran wrote:i had added a logic like this before

I don't know if I need to write that much code yet. I'm still thinking from a user perspective.

As a vendor and vending machine maker, I would like the vendor interface to be as simple as possible. Perhaps a simple LCD display and a few option buttons and numeric keypad.

I imagine the workflow like this:
1. Press "MAINT" button to enter maintenance mode
2. Press "REPORT" button to print report of current quantities
3. Press "FILL" button to fill up a tray to capacity, press "ENTER" or "CANCEL". Note down quantity displayed.
4. Optional: Press "FILL PART" button to partially fill a tray, enter quantity loaded then press "ENTER" or "CANCEL"
5. Optional: Press "REPORT" button to print report of current quantities after loading
6. Press "MAINT" button to exit maintenance mode

When you press the FILL button, the display will prompt you for a Tray # which you enter, and the machine will show how many products need to be added to fill it up completely. You note that number down in your stock sheet.

When you press the FILL PART button, the display will prompt you to enter the quantity you are adding. Then you press ENTER or CANCEL. The machine will display the amount you specified to add.

I would keep this in mind before I proceed to write more test and production code.

[s ravi chandran]

If we are considering filling up slots with fixed number of products, then can we add one more test case for that?

@Test
	public void fill_with_limited_product_should_return_number_of_items_added_to_empty_tray() throws Exception {
         // maybe add something like 
       int limitedProductQuantity = 3;
       int itemsAdded = testSubject.fill(FIRST_TRAY. limitedProductQuantity );

      assertThat(itemsAdded, is(3));
}

@Test
	public void fill_with_limited_product_should_return_number_of_items_added_to_partially_empty_tray() throws Exception {
           testSubject.fill(FIRST_TRAY);
		testSubject.dispense(FIRST_TRAY);

                //similar to previous testcase
           int limitedProductQuantity = 3;
          int itemsAdded = testSubject.fill(FIRST_TRAY. limitedProductQuantity );
          assertThat(itemsAdded, is(1));
          }

Will this be okay?

[Junilu Lacar]

s ravi chandran wrote:Yes, I guess, that will come with more understanding of application. Well, I did remove the logic later, as it was failing the partial fill case.

Here's my test case for filling a partially used up tray:

	@Test
	public void fill_should_return_number_of_items_added_to_partially_empty_tray() throws Exception {
		testSubject.fill(FIRST_TRAY);
		testSubject.dispense(FIRST_TRAY);
		
		int itemsAdded = testSubject.fill(FIRST_TRAY);
		assertThat(itemsAdded, is(1));
	}

Is that similar to what you had? The first two lines set the stage: you first fill up the tray, then dispense one product. This gets you a partially used up tray. The next two lines are filling up the tray to capacity again, then confirming that you only needed to replenish the one item that was dispensed earlier.

[s ravi chandran]

okay, got it.

[s ravi chandran]

yes it was similar but I was adding the quantity as a parameter.

[Junilu Lacar]

s ravi chandran wrote:If we are considering filling up slots with fixed number of products, then can we add one more test case for that?

@Test
	public void fill_with_limited_product_should_return_number_of_items_added_to_empty_tray() throws Exception {...
}

@Test
	public void fill_with_limited_product_should_return_number_of_items_added_to_partially_empty_tray() throws Exception {...
          }

Will this be okay?

Well, the names are still not good. And you messed up the formatting again.

Seriously, poorly formatted code is the easiest thing to fix so I get easily annoyed when programmers don't do it. In IDEs like Eclipse, it's a few keystrokes (CTRL-SHIFT-F or CMD-SHIFT-F)

Now back to the names. What does "fill with limited product" mean? I don't see that "with limited product" tells me anything useful or relevant.

How about "fill should confirm partial replenishment of a tray" and "fill_should_return_0_for_invalid_quantity"? That is, when you call fill(tray, quantity), it returns the number it added to its internal count of product. If the number entered was not logical, i.e. it would have resulted in a total that was greater than the actual capacity of the tray, then it would give back 0. Thinking back to the user interface and workflow, it would go something like this:

Vendor presses FILL PART button. Machine prompts with maximum number to enter to actually fill the tray completely. For example, FILL PART pressed, machine prompts "Enter quantity added (Max=4)" which means you can add up to 4 more products to replenish the tray. If the vendor accidentally presses the 7 button then ENTER, the machine will prompt again: "Invalid input. Enter quantity added (Max=4)" Returning 0 for invalid input makes the program logic easier to write to fit this workflow. If the vendor presses 3, then the machine can just confirm that with "3 items added to tray".

[s ravi chandran]

My code is formatted, it is just after posting, the spaces and the tabs do the trick. I had one question regarding the console option. if the user presses FILL PART, do we show him the current remaining quantity? or can he use the report option for that?

[Junilu Lacar]

s ravi chandran wrote:yes it was similar but I was adding the quantity as a parameter.

I suppose that's fine. My preference to have a simpler version that doesn't need additional input is simply that, a preference. I think it does make for simpler code for that particular workflow though.

[Junilu Lacar]

s ravi chandran wrote:My code is formatted, it is just after posting, the spaces and the tabs do the trick.

Ok, I'll let it slide this time ...

I had one question regarding the console option. if the user presses FILL PART, do we show him the current remaining quantity? or can he use the report option for that?

Well, put yourself in the guy's shoes. Does the remaining quantity really concern you when you're loading product into a tray? If it were me, it wouldn't. I just want to know how much product I need to fill up the tray. The report I print before loading up the machine has that other information anyway.

[s ravi chandran]

Junilu Lacar wrote:

s ravi chandran wrote:My code is formatted, it is just after posting, the spaces and the tabs do the trick.

Ok, I'll let it slide this time ...

I had one question regarding the console option. if the user presses FILL PART, do we show him the current remaining quantity? or can he use the report option for that?

Well, put yourself in the guy's shoes. Does the remaining quantity really concern you when you're loading product into a tray? If it were me, it wouldn't. I just want to know how much product I need to fill up the tray. The report I print before loading up the machine has that other information anyway.

Yes, i was asking that only, so we do show the user the remaining quantity to be filled.. ?

[Junilu Lacar]

s ravi chandran wrote:so we do show the user the remaining quantity to be filled.. ?

Yes. I imagine the prompt as being something like: "Enter # added (max=4)"

This is another thing to note: someone has to keep the big picture in mind. Pair or mob programming makes this easier to do. If you're solo programming, it requires a fair amount of mental agility and discipline to switch from low-level thinking to high-level thinking. When you pair- or mob-program, someone can drive (be at the keyboard) and think of low-level implementation details, while other people can think high level and keep the big picture in focus.

[s ravi chandran]

Ok, we have the stories ready, I think we can start test codes. regarding the two test cases i was telling about, limited_product_quantity doesn't give useful info as to the purpose of the test?