SellOneItemTest.java

package ca.jbrains.pos.test;

import static org.junit.Assert.*;

import java.util.*;

import org.junit.*;

public class SellOneItemTest {
	private Display display;
	private DisplayFormatter displayFormatter;
	private Catalogue catalogue;
	private Sale sale;
	private Product product_1, product_2, product_3;

	@Before
	public void setup() {
		displayFormatter = new DisplayFormatter();
		display = new Display(displayFormatter);
		catalogue = new Catalogue();
		product_1 = new Product("123", "$12.50");
		product_2 = new Product("456", "$20.00");
		product_3 = new Product("789", "$0.00");
		catalogue.addProduct(product_1);
		catalogue.addProduct(product_2);
		catalogue.addProduct(product_3);
		sale = new Sale(display, catalogue);
	}

	public void afterScanningTheDisplayShows(String code, String message) {
		sale.onBarcode(code);
		assertEquals(message, display.displayFormattedText());
	}
	
	@Test
	public void productFound() throws Exception {
		afterScanningTheDisplayShows(product_1.barcode(), product_1.price());
	}

	@Test
	public void anotherProductFound() throws Exception {
		afterScanningTheDisplayShows(product_2.barcode(), product_2.price());
	}
	
	@Test
	public void productSoldWithPriceOfZeroDisplayedAsFree() throws Exception {
		afterScanningTheDisplayShows(product_3.barcode(), "FREE");
	}

	@Test
	public void productNotFound() throws Exception {
		afterScanningTheDisplayShows("999", "No product with barcode 999");
	}

	@Test
	public void emptyBarcode() throws Exception {
		afterScanningTheDisplayShows("", "Scanning error: empty barcode");
	}
	
	@Test
	public void nullBarcode() throws Exception {
		afterScanningTheDisplayShows(null, "Scanning error: empty barcode");
	}
}

class Sale {
	private Display display;
	private Catalogue catalogue;

	public Sale(Display display, Catalogue catalogue) {
		this.display = display;
		this.catalogue = catalogue;
	}

	public void onBarcode(String barcode) {
		String text;
		if (invalid(barcode)) {
			text = "Scanning error: empty barcode";
		} else {
			text = catalogue.priceFor(barcode);
		}
		display.setText(text);
	}

	private boolean invalid(String barcode) {
		return barcode == null || "".equals(barcode);
	}

}

class Catalogue {
	private Products products = new Products();

	public Catalogue() {}

	public String priceFor(String barcode) {
		if (products.productExistsFor(barcode))
			return products.priceForProductWith(barcode);
		else
			return "No product with barcode " + barcode;
	}

	public void addProduct(Product product) {
		products.addProduct(product);
	}
	
}

class Products {
	private Map<String, Product> productsByBarcode;

	public Products() {
		productsByBarcode = new HashMap<String, Product>();
	}

	String priceForProductWith(String barcode) {
		return productsByBarcode.get(barcode).price();
	}

	public boolean productExistsFor(String barcode) {
		return productsByBarcode.containsKey(barcode);
	}
	
	public void addProduct(Product product) {
		productsByBarcode.put(product.barcode(), product);
	}
}

class Product {
	private String barcode;
	private String price;
	
	public Product(String barcode, String price) {
		this.barcode = barcode;
		this.price = price;
	}

	public String barcode() {
		return barcode;
	}

	public String price() {
		return price;
	}
}

class DisplayFormatter {
	public String format(String text) {
		if (shouldDisplayAsFree(text))
			return "FREE";
		else
			return text;
	}

	private boolean shouldDisplayAsFree(String text) {
		return "$0.00".equals(text);
	}
}

class Display {
	private String text;
	private DisplayFormatter displayFormatter;
	
	public Display(DisplayFormatter displayFormatter) {
		this.displayFormatter = displayFormatter;
	}

	public void setText(String text) {
		this.text = text;
	}

	public String displayFormattedText() {
		return displayFormatter.format(text);
	}
}

Thanks.

Given comments 4 to 6 I can see that I've got some feature envy. I will move those better named methods to the correct classes. Given that, it looks like there is still another class wanting to come out from the Catalgoue class.

Test duplication: My thoughts on this are that I should remove the tests I added to drive the DisplayRules class. I could also remove one of either productFound or anotherProductFound. Is it normal to remove tests that "essentially" duplicate behaviour once that logic has been driven out?

Set up of the Catalogue I'll need to think about some more. addProducts method perhaps... I'm not sure.

I had a horrible feeling about using DisplayRules inside Display originally. I'm thinking the text should be formatted by the DisplayRules class before being sent to the display. The fact I used the word formatted also suggests that I should rename DisplayRules to DisplayFormatter.

I'm also thinking that I should be testing the units more instead of doing, what is essentially, integration testing with the Sale class. This little problem has quite a few options! :)

I'll make these changes soon, this comment was part answering your questions, part me trying not to forget ideas...

Is it normal to remove tests that "essentially" duplicate behaviour once that logic has been driven out?

I find it common, yes. Regarding whether to remove the tests, I don't always act the same way. The Novice rule is "keep the tests", and when I first started practising test-first programming, I usually kept the tests, even when they were duplicates. Now I tend to remove the duplicate tests when I feel confident and retain them when I feel less confident.

Set up of the Catalogue I'll need to think about some more. addProducts method perhaps... I'm not sure.

What could you do to avoid addProducts() or addProduct() entirely?

I had a horrible feeling about using DisplayRules inside Display originally. I'm thinking the text should be formatted by the DisplayRules class before being sent to the display. The fact I used the word formatted also suggests that I should rename DisplayRules to DisplayFormatter.

That sounds reasonable, and I recommend you try it. This looks like a textbook case of inverting the dependency.

I'm also thinking that I should be testing the units more instead of doing, what is essentially, integration testing with the Sale class. This little problem has quite a few options! :)

Pick one and try it; that's the beauty of exercises like these.