From 4a5ea2ee3ac89ecb5e77c761ffdf662f56a266fd Mon Sep 17 00:00:00 2001 From: Frida Tveit Date: Sat, 21 Jan 2017 13:15:10 +0000 Subject: [PATCH 1/3] 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. --- .../grade-school/src/example/java/School.java | 34 +++++++------- .../src/test/java/SchoolTest.java | 44 ++++++++++--------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/exercises/grade-school/src/example/java/School.java b/exercises/grade-school/src/example/java/School.java index c05abe031..777ff39fc 100644 --- a/exercises/grade-school/src/example/java/School.java +++ b/exercises/grade-school/src/example/java/School.java @@ -1,38 +1,36 @@ -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> database = new HashMap>(); + private final Map> database = new HashMap<>(); - public Map> db() { - // Leaks internal storage to caller - return database; + public int numberOfStudents() { + int result = 0; + for (List studentsInGrade: database.values()) { + result += studentsInGrade.size(); + } + return result; } public void add(String student, int grade) { - Set students = grade(grade); + List students = grade(grade); students.add(student); } - public Set grade(int grade) { + public List grade(int grade) { // Leaks internal storage to caller if (!database.containsKey(grade)) { - database.put(grade, new TreeSet()); + database.put(grade, new LinkedList<>()); } return database.get(grade); } - public Map> sort() { - Map> sortedStudents = new HashMap>(); + public Map> studentsByGradeAlphabetical() { + Map> sortedStudents = new HashMap<>(); for (Integer grade : database.keySet()) { - // Relies on using a TreeSet internally - List sortedGrade = new ArrayList(database.get(grade)); - sortedStudents.put(grade, sortedGrade); + List studentsInGrade = database.get(grade); + Collections.sort(studentsInGrade); + sortedStudents.put(grade, studentsInGrade); } return sortedStudents; } diff --git a/exercises/grade-school/src/test/java/SchoolTest.java b/exercises/grade-school/src/test/java/SchoolTest.java index ba7df39d6..1244c2043 100644 --- a/exercises/grade-school/src/test/java/SchoolTest.java +++ b/exercises/grade-school/src/test/java/SchoolTest.java @@ -7,22 +7,23 @@ 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 +34,8 @@ public void addsMoreStudentsInSameGrade() { 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 +44,30 @@ public void addsStudentsInMultipleGrades() { 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 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 +81,6 @@ public void sortsSchool() { 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); } } From feecb1c852802ae68681b73b8d3069079e95f4ab Mon Sep 17 00:00:00 2001 From: Frida Tveit Date: Sat, 21 Jan 2017 14:19:54 +0000 Subject: [PATCH 2/3] Added tests for protection agains mutation of the list object --- .../grade-school/src/example/java/School.java | 7 +++-- .../src/test/java/SchoolTest.java | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/exercises/grade-school/src/example/java/School.java b/exercises/grade-school/src/example/java/School.java index 777ff39fc..36eb91c62 100644 --- a/exercises/grade-school/src/example/java/School.java +++ b/exercises/grade-school/src/example/java/School.java @@ -13,12 +13,15 @@ public int numberOfStudents() { } public void add(String student, int grade) { - List students = grade(grade); + List students = fetchGradeFromDatabase(grade); students.add(student); } public List grade(int grade) { - // Leaks internal storage to caller + return new ArrayList<>(fetchGradeFromDatabase(grade)); + } + + private List fetchGradeFromDatabase(int grade) { if (!database.containsKey(grade)) { database.put(grade, new LinkedList<>()); } diff --git a/exercises/grade-school/src/test/java/SchoolTest.java b/exercises/grade-school/src/test/java/SchoolTest.java index 1244c2043..7bd2fc281 100644 --- a/exercises/grade-school/src/test/java/SchoolTest.java +++ b/exercises/grade-school/src/test/java/SchoolTest.java @@ -1,6 +1,9 @@ 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; @@ -83,4 +86,30 @@ public void sortsSchool() { sortedStudents.put(3, Arrays.asList("Kyle")); assertEquals(school.studentsByGradeAlphabetical(), sortedStudents); } + + @Ignore + @Test + public void modifyingFetchedGradeShouldNotModifyInternalDatabase() { + String shouldNotBeAdded = "Should not be added to school"; + int grade = 1; + + List 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 listWhichShouldNotBeAdded = new ArrayList<>(); + listWhichShouldNotBeAdded.add(studentWhichShouldNotBeAdded); + + Map> sortedStudents = school.studentsByGradeAlphabetical(); + sortedStudents.put(grade,listWhichShouldNotBeAdded); + + assertThat(school.studentsByGradeAlphabetical().get(grade), not(hasItem(studentWhichShouldNotBeAdded))); + } } From c3b73462674bbfc9246aa3bb9cd792da28c24af5 Mon Sep 17 00:00:00 2001 From: Dave Thomas Date: Sun, 22 Jan 2017 22:49:47 -0800 Subject: [PATCH 3/3] Issue #253: Make the sort test more generic. This to allow a broader range of implementations. Currently users are hindered into return the type HashMap. This test update will check the order of any Map that maps to any Collection value type. This opens the doors for more possible types of solutions. --- exercises/grade-school/build.gradle | 1 + .../src/test/java/SchoolTest.java | 25 +++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/exercises/grade-school/build.gradle b/exercises/grade-school/build.gradle index d019b23c7..7bd925eaf 100644 --- a/exercises/grade-school/build.gradle +++ b/exercises/grade-school/build.gradle @@ -7,6 +7,7 @@ repositories { } dependencies { + testCompile group: 'org.hamcrest', name: 'hamcrest-library', version: '1.3' testCompile "junit:junit:4.12" } test { diff --git a/exercises/grade-school/src/test/java/SchoolTest.java b/exercises/grade-school/src/test/java/SchoolTest.java index ba7df39d6..7b1d96290 100644 --- a/exercises/grade-school/src/test/java/SchoolTest.java +++ b/exercises/grade-school/src/test/java/SchoolTest.java @@ -1,11 +1,12 @@ import org.junit.Ignore; import org.junit.Test; -import java.util.Arrays; +import java.util.Collection; import java.util.HashMap; -import java.util.List; import java.util.Map; +import org.hamcrest.Matcher; +import static org.hamcrest.collection.IsIterableContainingInOrder.*; import static org.hamcrest.CoreMatchers.*; import static org.junit.Assert.*; @@ -69,14 +70,22 @@ public void getsStudentsInEmptyGrade() { @Ignore @Test public void sortsSchool() { + school.add("Kyle", 4); + school.add("Zed", 4); + school.add("Adam", 4); school.add("Jennifer", 4); school.add("Kareem", 6); school.add("Christopher", 4); - school.add("Kyle", 3); - Map> sortedStudents = new HashMap>(); - sortedStudents.put(6, Arrays.asList("Kareem")); - sortedStudents.put(4, Arrays.asList("Christopher", "Jennifer")); - sortedStudents.put(3, Arrays.asList("Kyle")); - assertEquals(school.sort(), sortedStudents); + school.add("Kylie", 3); + Map sortedStudents = new HashMap(); + sortedStudents.put(6, contains("Kareem")); + sortedStudents.put(4, contains("Adam", "Christopher", "Jennifer", "Kyle", "Zed")); + sortedStudents.put(3, contains("Kylie")); + + Map schoolStudents = school.sort(); + for (Map.Entry entry : sortedStudents.entrySet()) { + + assertThat((Collection) schoolStudents.get(entry.getKey()), entry.getValue()); + } } }