Issue #253: Tried to improve grade school structure. (#259)

* Issue #253: Tried to improve grade school structure. Removed .db() function, added numberOfStudents functions and renamed the sort function to studentsByGradeAlphabetical. Added test to check that grade() gets the students in the order they were inserted. Removed getsStudentsInAGrade() test as what it was testing was already being covered by the tests before it.

* Added tests for protection agains mutation of the list object

exercises/grade-school/src/example/java/School.java

@@ -1,38 +1,39 @@
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.TreeSet;
+import java.util.*;
 
 public class School {
 
-  private final Map<Integer, Set<String>> database = new HashMap<Integer, Set<String>>();
+  private final Map<Integer, List<String>> database = new HashMap<>();
 
-  public Map<Integer, Set<String>> db() {
-    // Leaks internal storage to caller
-    return database;
+  public int numberOfStudents() {
+    int result = 0;
+    for (List<String> studentsInGrade: database.values()) {
+      result += studentsInGrade.size();
+    }
+    return result;
   }
 
   public void add(String student, int grade) {
-    Set<String> students = grade(grade);
+    List<String> students = fetchGradeFromDatabase(grade);
     students.add(student);
   }
 
-  public Set<String> grade(int grade) {
-    // Leaks internal storage to caller
+  public List<String> grade(int grade) {
+    return new ArrayList<>(fetchGradeFromDatabase(grade));
+  }
+
+  private List<String> fetchGradeFromDatabase(int grade) {
     if (!database.containsKey(grade)) {
-      database.put(grade, new TreeSet<String>());
+      database.put(grade, new LinkedList<>());
     }
     return database.get(grade);
   }
 
-  public Map<Integer, List<String>> sort() {
-    Map<Integer, List<String>> sortedStudents = new HashMap<Integer, List<String>>();
+  public Map<Integer, List<String>> studentsByGradeAlphabetical() {
+    Map<Integer, List<String>> sortedStudents = new HashMap<>();
     for (Integer grade : database.keySet()) {
-      // Relies on using a TreeSet internally
-      List<String> sortedGrade = new ArrayList<String>(database.get(grade));
-      sortedStudents.put(grade, sortedGrade);
+      List<String> studentsInGrade = database.get(grade);
+      Collections.sort(studentsInGrade);
+      sortedStudents.put(grade, studentsInGrade);
     }
     return sortedStudents;
   }

exercises/grade-school/src/test/java/SchoolTest.java

@@ -1,28 +1,32 @@
 import org.junit.Ignore;
 import org.junit.Test;
 
+import java.lang.Integer;
+import java.util.*;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
 import static org.hamcrest.CoreMatchers.*;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertEquals;
 
 public class SchoolTest {
   private final School school = new School();
 
-
   @Test
   public void startsWithNoStudents() {
-    assertTrue(school.db().isEmpty());
+    assertThat(school.numberOfStudents(), is(0));
   }
 
   @Ignore
   @Test
   public void addsStudents() {
     school.add("Aimee", 2);
-    assertThat(school.db().get(2), hasItem("Aimee"));
+    assertThat(school.grade(2), hasItem("Aimee"));
   }
 
   @Ignore
@@ -33,8 +37,8 @@ public class SchoolTest {
     school.add("Blair", grade);
     school.add("Paul", grade);
 
-    assertThat(school.db().get(grade).size(), is(3));
-    assertThat(school.db().get(grade), allOf(hasItem("James"), hasItem("Blair"), hasItem("Paul")));
+    assertThat(school.grade(grade).size(), is(3));
+    assertThat(school.grade(grade), allOf(hasItem("James"), hasItem("Blair"), hasItem("Paul")));
   }
 
   @Ignore
@@ -43,27 +47,30 @@ public class SchoolTest {
     school.add("Chelsea", 3);
     school.add("Logan", 7);
 
-    assertThat(school.db().size(), is(2));
-    assertThat(school.db().get(3).size(), is(1));
-    assertThat(school.db().get(3), hasItem("Chelsea"));
-    assertThat(school.db().get(7).size(), is(1));
-    assertThat(school.db().get(7), hasItem("Logan"));
+    assertThat(school.numberOfStudents(), is(2));
+    assertThat(school.grade(3).size(), is(1));
+    assertThat(school.grade(3), hasItem("Chelsea"));
+    assertThat(school.grade(7).size(), is(1));
+    assertThat(school.grade(7), hasItem("Logan"));
   }
 
   @Ignore
   @Test
-  public void getsStudentsInAGrade() {
-    school.add("Franklin", 5);
-    school.add("Bradley", 5);
-    school.add("Jeff", 1);
-    assertThat(school.grade(5).size(), is(2));
-    assertThat(school.grade(5), allOf(hasItem("Franklin"), hasItem("Bradley")));
+  public void getsStudentsInEmptyGrade() {
+    assertTrue(school.grade(1).isEmpty());
   }
 
   @Ignore
   @Test
-  public void getsStudentsInEmptyGrade() {
-    assertTrue(school.grade(1).isEmpty());
+  public void gradeReturnsStudentsInTheOrderTheyWereInserted() {
+    int grade = 4;
+    school.add("Bartimaeus", grade);
+    school.add("Nathaniel", grade);
+    school.add("Faquarl", grade);
+    List<String> studentsInGrade = school.grade(grade);
+    assertThat(studentsInGrade.get(0), is("Bartimaeus"));
+    assertThat(studentsInGrade.get(1), is("Nathaniel"));
+    assertThat(studentsInGrade.get(2), is("Faquarl"));
   }
 
   @Ignore
@@ -77,6 +84,32 @@ public class SchoolTest {
     sortedStudents.put(6, Arrays.asList("Kareem"));
     sortedStudents.put(4, Arrays.asList("Christopher", "Jennifer"));
     sortedStudents.put(3, Arrays.asList("Kyle"));
-    assertEquals(school.sort(), sortedStudents);
+    assertEquals(school.studentsByGradeAlphabetical(), sortedStudents);
+  }
+
+  @Ignore
+  @Test
+  public void modifyingFetchedGradeShouldNotModifyInternalDatabase() {
+    String shouldNotBeAdded = "Should not be added to school";
+    int grade = 1;
+
+    List<String> students = school.grade(grade);
+    students.add(shouldNotBeAdded);
+
+    assertThat(school.grade(grade), not(hasItem(shouldNotBeAdded)));
+  }
+
+  @Ignore
+  @Test
+  public void modifyingSortedStudentsShouldNotModifyInternalDatabase() {
+    int grade = 2;
+    String studentWhichShouldNotBeAdded = "Should not be added";
+    List<String> listWhichShouldNotBeAdded = new ArrayList<>();
+    listWhichShouldNotBeAdded.add(studentWhichShouldNotBeAdded);
+
+    Map<Integer, List<String>> sortedStudents = school.studentsByGradeAlphabetical();
+    sortedStudents.put(grade,listWhichShouldNotBeAdded);
+
+    assertThat(school.studentsByGradeAlphabetical().get(grade), not(hasItem(studentWhichShouldNotBeAdded)));
   }
 }