Repository: exercism/java
Issue: book-store: Example implementation is incorrect

The description of the exercise describes a a scenario that is also a [test case](https://github.com/exercism/xjava/blob/master/exercises/book-store/src/test/java/BookstoreTest.java#L70-L74) where two groups of 4 are cheaper than two groups of 5 and 3. The example implementation always starts with the biggest possible group and does not try all other combinations to really find the cheapest price.

The problem why the tests don't find this is the incorrect use of the delta parameter in the assert method.
```java
List<Integer> books = new ArrayList<>(Arrays.asList(1,1,2,2,3,3,4,5));
Bookstore bookstore = new Bookstore(books);
assertEquals(51.20,bookstore.calculateTotalCost(),2);
```

While `calculateTotalCost()` actually returns the wrong price of `51.6` because of the `delta=2` even `53.2` or `49.2` would be considered equal.

> delta - the maximum delta between expected and actual for which both numbers are still considered equal. 

Maybe `delta` should better be something like `0.001` so people really implement the correct algorithm.

Changed files:
exercises/book-store/src/example/java/Bookstore.java, exercises/book-store/src/test/java/BookstoreTest.java

diff --git a/exercises/book-store/src/example/java/Bookstore.java b/exercises/book-store/src/example/java/Bookstore.java
index 54908ed6..48b27e7f 100644
--- a/exercises/book-store/src/example/java/Bookstore.java
+++ b/exercises/book-store/src/example/java/Bookstore.java
@@ -1,61 +1,57 @@
-import java.awt.print.Printable;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.List;
 import java.util.stream.Collectors;
 
-public class Bookstore{
+class Bookstore {
 
-	private static int BOOK_PRICE = 8, MAX_GROUP_SIZE = 5;
-	private List<Integer> books;
-	private static double[] DISCOUNT_TIERS = {0,5,10,20,25};   	
+  private static final int BOOK_PRICE = 8, MAX_GROUP_SIZE = 5;
 
-	public Bookstore (List<Integer> books){
-		this.books = books;
-	}	
-	
+  private static double[] DISCOUNT_TIERS = {0, 5, 10, 20, 25};
 
+  private List<Integer> books;
 
+  Bookstore(List<Integer> books) {
+    this.books = books;
+  }
 
-	public double calculateTotalCost(){
-		return calculateTotalCost(this.books,0);
-	}
+  double calculateTotalCost() {
+    return calculateTotalCost(this.books, 0);
+  }
 
-	private double calculateTotalCost (List<Integer> books,double priceSoFar ){
-		double minPrice = Double.MAX_VALUE;
+  private double calculateTotalCost(List<Integer> books, double priceSoFar) {
+    if (books.size() == 0) {
+      return priceSoFar;
+    }
 
+    List<Integer> availableBookNumbers = books.stream()
+        .distinct()
+        .collect(Collectors.toList());
 
-		if(books.size() == 0){
-			return priceSoFar;
-		}
+    double minPrice = Double.MAX_VALUE;
 
-		List<Integer> groups = (ArrayList<Integer>) books.stream().distinct().collect(Collectors.toList());
+    for (int i = 0; i < availableBookNumbers.size(); i++) {
+      List<Integer> newGroupBooks = new ArrayList<>(availableBookNumbers.subList(0, i + 1));
+      List<Integer> remainingBooks = new ArrayList<>(books);
 
-		
-		double price = 0;
+      for (final Integer newGroupBook : newGroupBooks) {
+        //noinspection UseBulkOperation - we want to remove _one_ of each book number, not _all_ of each book number.
+        remainingBooks.remove(newGroupBook);
+      }
 
-		for(int i = 0;i<groups.size();i++){
-			books.remove(groups.get(i));	
-		}
-				
-		try {
-			price = calculateTotalCost(books,priceSoFar + costPerGroup(groups.size()));
-		} catch (Exception e) {
-			e.printStackTrace();
-		}
-		
-		minPrice = Math.min(minPrice, price);
-		return minPrice;
+      double price = calculateTotalCost(remainingBooks, priceSoFar + costOfGroupSize(newGroupBooks.size()));
 
+      minPrice = Math.min(minPrice, price);
+    }
 
-	}
+    return minPrice;
+  }
 
-	private double costPerGroup(int groupSize) throws Exception{
-		if (groupSize < 1 || groupSize > MAX_GROUP_SIZE){
-        		throw new Exception("Invalid group size : " + groupSize );
-     		}
-    		return BOOK_PRICE * groupSize * (100 - DISCOUNT_TIERS[groupSize-1])/100;
-	}
+  private double costOfGroupSize(int groupSize) {
+    if (groupSize < 1 || groupSize > MAX_GROUP_SIZE) {
+      throw new IllegalStateException("Invalid group size : " + groupSize);
+    }
 
+    return BOOK_PRICE * groupSize * (100 - DISCOUNT_TIERS[groupSize - 1]) / 100;
+  }
 
 }
diff --git a/exercises/book-store/src/test/java/BookstoreTest.java b/exercises/book-store/src/test/java/BookstoreTest.java
index 8552d393..89389648 100644
--- a/exercises/book-store/src/test/java/BookstoreTest.java
+++ b/exercises/book-store/src/test/java/BookstoreTest.java
@@ -1,5 +1,6 @@
 import static org.junit.Assert.assertEquals;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -7,112 +8,112 @@ import java.util.Arrays;
 import org.junit.Ignore;
 import org.junit.Test;
 
-public class BookstoreTest{
-
-	@Test
-	public void onlyASingleBook(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(8,bookstore.calculateTotalCost(),2);
-	}
-	
-	@Ignore
-	@Test
-	public void twoOfSameBook(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,1));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(16,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void emptyBasket(){
-		List<Integer> books = new ArrayList<>();
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(0,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void twoDifferentBooks(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,2));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(15.20,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void threeDifferentBooks(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,2,3));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(21.6,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void fourDifferentBooks(){
-		ArrayList<Integer> books = new ArrayList<>(Arrays.asList(1,2,3,4));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(25.6,bookstore.calculateTotalCost(),2);
-
-	}
-
-	@Ignore
-	@Test
-	public void fiveDifferentBooks(){
-		ArrayList<Integer> books = new ArrayList<>(Arrays.asList(1,2,3,4,5));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(30,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void twoGroupsOfFourIsCheaperThanGroupOfFivePlusGroupOfThree(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,1,2,2,3,3,4,5));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(51.20,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void groupOfFourPlusGroupOfTwoIsCheaperThanTwoGroupsOfThree(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,1,2,2,3,4));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(40.8,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void twoEachOfFirst4BooksAnd1CopyEachOfRest(){
-		Integer[] p = {1,1,2,2,3,3,4,4,5};
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,1,2,2,3,3,4,4,5));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(55.60,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void twoCopiesOfEachBook(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,1,2,2,3,3,4,4,5,5));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(60.00,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void threeCopiesOfFirstBookAnd2EachOfRemaining(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,1,2,2,3,3,4,4,5,5,1));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(68.00,bookstore.calculateTotalCost(),2);
-	}
-
-	@Ignore
-	@Test
-	public void threeEachOFirst2BooksAnd2EachOfRemainingBooks(){
-		List<Integer> books = new ArrayList<>(Arrays.asList(1,1,2,2,3,3,4,4,5,5,1,2));
-		Bookstore bookstore = new Bookstore(books);
-		assertEquals(75.20,bookstore.calculateTotalCost(),2);
-	}
+public class BookstoreTest {
+
+  // This is sufficient accuracy since we're handling currency values, which should be equal to within 2 decimal places.
+  private static final double EQUALITY_TOLERANCE = 0.001;
+
+  @Test
+  public void onlyASingleBook() {
+    List<Integer> books = new ArrayList<>(Collections.singletonList(1));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(8, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void twoOfSameBook() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 1));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(16, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void emptyBasket() {
+    List<Integer> books = new ArrayList<>();
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(0, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void twoDifferentBooks() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 2));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(15.20, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void threeDifferentBooks() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 2, 3));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(21.6, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void fourDifferentBooks() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 2, 3, 4));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(25.6, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void fiveDifferentBooks() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 2, 3, 4, 5));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(30, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void twoGroupsOfFourIsCheaperThanGroupOfFivePlusGroupOfThree() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 1, 2, 2, 3, 3, 4, 5));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(51.20, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void groupOfFourPlusGroupOfTwoIsCheaperThanTwoGroupsOfThree() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 1, 2, 2, 3, 4));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(40.8, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void twoEachOfFirst4BooksAnd1CopyEachOfRest() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 1, 2, 2, 3, 3, 4, 4, 5));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(55.60, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void twoCopiesOfEachBook() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 1, 2, 2, 3, 3, 4, 4, 5, 5));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(60.00, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void threeCopiesOfFirstBookAnd2EachOfRemaining() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 1));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(68.00, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
+
+  @Ignore
+  @Test
+  public void threeEachOFirst2BooksAnd2EachOfRemainingBooks() {
+    List<Integer> books = new ArrayList<>(Arrays.asList(1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 1, 2));
+    Bookstore bookstore = new Bookstore(books);
+    assertEquals(75.20, bookstore.calculateTotalCost(), EQUALITY_TOLERANCE);
+  }
 
 }
-