-
-
Save flaviusstef/725342 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
package ca.jbrains.pos.test; | |
import org.junit.*; | |
import static org.junit.Assert.*; | |
import java.util.*; | |
public class SellOneItemTest { | |
private POSDisplay display = new POSDisplay(); | |
private Sale sale; | |
public static class Sale { | |
private POSDisplay display; | |
private Map<String, String> pricesByBarcode = new HashMap<String, String> (); | |
public Sale(POSDisplay d, Map<String, String> pricesByBarcode) { | |
if (pricesByBarcode == null) { | |
throw new RuntimeException("Invalid price list"); | |
} | |
display = d; | |
this.pricesByBarcode = pricesByBarcode; | |
} | |
public void onBarcode(String barcode) { | |
if ("".equals(barcode)) { | |
display.onEmptyBarcode(); | |
return; | |
} | |
if (!pricesByBarcode.containsKey(barcode)) { | |
display.onInvalidBarcode(barcode); | |
return; | |
} | |
display.showPrice(pricesByBarcode.get(barcode)); | |
} | |
} | |
public static class Display { | |
private String text; | |
public void setText(String text) { | |
this.text = text; | |
} | |
public String getText() { | |
return text; | |
} | |
} | |
public static class POSDisplay extends Display { | |
public void onEmptyBarcode() { | |
setText("Scanning error: empty barcode"); | |
} | |
public void onInvalidBarcode(String barcode) { | |
setText("No product with barcode " + barcode); | |
} | |
public void showPrice(String price) { | |
setText(price.equals("$0.00") ? "FREE" : price); | |
} | |
} | |
@SuppressWarnings("serial") | |
@Before | |
public void setUp() { | |
sale = new Sale(display, new HashMap<String, String>() { | |
{ | |
put("123", "$12.50"); | |
put("456", "$20.00"); | |
put("333", "$0.00"); | |
} | |
}); | |
} | |
@Test | |
public void productFound() throws Exception { | |
assertForBarcodeDisplayShows("123", "$12.50"); | |
} | |
@Test | |
public void anotherProductFound() throws Exception { | |
assertForBarcodeDisplayShows("456", "$20.00"); | |
} | |
@Test | |
public void productNotFound() throws Exception { | |
assertForBarcodeDisplayShows("999", "No product with barcode 999"); | |
} | |
@Test | |
public void emptyBarcode() throws Exception { | |
assertForBarcodeDisplayShows("", "Scanning error: empty barcode"); | |
} | |
@Test | |
public void freeProductHasDistinctFormat() throws Exception { | |
assertForBarcodeDisplayShows("333", "FREE"); | |
} | |
@Test(expected=RuntimeException.class) | |
public void nullProductListIsInvalid() { | |
new Sale(display, null); | |
} | |
private void assertForBarcodeDisplayShows(String barcode, String message) { | |
sale.onBarcode(barcode); | |
assertEquals(message, display.getText()); | |
} | |
} |
I decided to make a small number of improvements at https://gist.github.com/731349 to show you an example of how I approach the code you're provided here. You can clone the entire repository to step through the commits and examine the differences.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
First, my own observations, and then my comments on your comments.
this.x = x;
in constructors, then I recommend doing it uniformly.null
pricesByBarcode
to describe the problem more precisely.Sale
to require non-null
apricesByBarcode
even though there is a code path throughonBarcode()
that doesn't requirepricesByBarcode
. If you sometimes need one and sometimes don't, what might that say about the design?POSDisplay
, which I quite like.POSDisplay
inconsistently. You have the imperativeshowPrice()
but the event handlingonInvalidBarcode()
andonEmptyBarcode()
.onInvalidBarcode()
, but is a barcode not in our catalog truly invalid? What other method name would describe this special case more accurately?Display
andPOSDisplay
as separate classes?"FREE"
now appears in the view, which I find quite sensible. Now look at the methodshowPrice(String price)
. Should we model a price as text?Otherwise, I like what you've done, and especially
assertForBarcodeDisplayShows()
.Regarding whether to write tests directly for
POSDisplay
, I think you can survive without it for a moment.