Merge pull request #145 from butteredyakiimo/branch-quality-and-testing

Add more tests and check code quality

docs/diagrams/StatsAvailSelfInvDiagram.puml

@@ -12,6 +12,11 @@ activate StatsAvailCommand
 StatsAvailCommand --> StatsAvailCommand : availableFosterers
 deactivate StatsAvailCommand
 
+StatsAvailCommand -> StatsAvailCommand : calculatePercentage(availableCount, total)
+activate StatsAvailCommand
+StatsAvailCommand --> StatsAvailCommand
+deactivate StatsAvailCommand
+
 StatsAvailCommand -> StatsAvailCommand : getAbleDogCount(availableFosterers)
 activate StatsAvailCommand
 StatsAvailCommand --> StatsAvailCommand : ableDogCount
@@ -22,11 +27,6 @@ activate StatsAvailCommand
 StatsAvailCommand --> StatsAvailCommand : ableCatCount
 deactivate StatsAvailCommand
 
-StatsAvailCommand -> StatsAvailCommand : calculatePercentage(availableCount, total)
-activate StatsAvailCommand
-StatsAvailCommand --> StatsAvailCommand
-deactivate StatsAvailCommand
-
 StatsAvailCommand -> StatsAvailCommand : calculatePercentage(ableDogCount, availableCount)
 activate StatsAvailCommand
 StatsAvailCommand --> StatsAvailCommand

src/main/java/seedu/address/commons/core/index/Indices.java

@@ -30,7 +30,7 @@ private Indices(SortedSet<Index> zeroBasedIndices) {
      * Creates a new {@code Indices} using zero-based indices.
      */
     public static Indices fromZeroBased(int[] zeroBasedIndices) {
-        assert(zeroBasedIndices != null);
+        assert (zeroBasedIndices != null);
         SortedSet<Index> result = new TreeSet<>();
 
         for (int index : zeroBasedIndices) {
@@ -44,7 +44,7 @@ public static Indices fromZeroBased(int[] zeroBasedIndices) {
      * Creates a new {@code Indices} using one-based indices.
      */
     public static Indices fromOneBased(int[] oneBasedIndices) {
-        assert(oneBasedIndices != null);
+        assert (oneBasedIndices != null);
         SortedSet<Index> result = new TreeSet<>();
 
         for (int index : oneBasedIndices) {

src/main/java/seedu/address/logic/commands/StatsAvailCommand.java

@@ -8,7 +8,6 @@
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 import seedu.address.model.person.AnimalType;
-import seedu.address.model.person.Availability;
 import seedu.address.model.person.Person;
 
 /**
@@ -28,9 +27,7 @@ public class StatsAvailCommand extends StatsCommand {
      */
     protected List<Person> getAvailableFosterers(List<Person> fosterers) {
         return fosterers.stream()
-                .filter(fosterer ->
-                        fosterer.getAvailability()
-                                .equals(Availability.AVAILABLE))
+                .filter(Person::isAvailableFosterer)
                 .collect(Collectors.toList());
     }
 
@@ -44,6 +41,7 @@ protected int getAbleDogCount(List<Person> fosterers) {
                                 .equals(AnimalType.ABLE_DOG))
                 .count();
     }
+
     /**
      * Returns the number of fosterers from the given list who can foster cats.
      */

src/main/java/seedu/address/logic/commands/StatsCommand.java

@@ -19,7 +19,8 @@ public abstract class StatsCommand extends Command {
             + "Please add some fosterers!";
 
     /**
-     * Calculates the percentage of a numerator and denominator.
+     * Calculates percentage using a numerator and denominator.
+     * Denominator must be a non-zero value.
      */
     protected static double calculatePercentage(int num, int denom) {
         assert(denom != 0);

src/main/java/seedu/address/logic/commands/StatsCurrentCommand.java

@@ -8,7 +8,6 @@
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
 import seedu.address.model.person.AnimalType;
-import seedu.address.model.person.Availability;
 import seedu.address.model.person.Person;
 
 /**
@@ -24,17 +23,10 @@ public class StatsCurrentCommand extends StatsCommand {
 
     /**
      * Returns a list of fosterers who are currently fostering.
-     * A fosterer is considered currently fostering if and only if they are not available, and
-     * currently has a dog or has a cat in their care, and animal name is not nil.
      */
     protected List<Person> getCurrentFosterers(List<Person> fosterers) {
         return fosterers.stream()
-                .filter(fosterer ->
-                        fosterer.getAvailability()
-                                .equals(Availability.NOT_AVAILABLE)
-                                && !fosterer.getAnimalName().fullName.equals(Person.NIL_WORD)
-                                && fosterer.getAnimalType().equals(AnimalType.CURRENT_DOG)
-                                || fosterer.getAnimalType().equals(AnimalType.CURRENT_CAT))
+                .filter(Person::isCurrentFosterer)
                 .collect(Collectors.toList());
     }
 

src/main/java/seedu/address/logic/commands/StatsHousingCommand.java

@@ -26,29 +26,30 @@ public class StatsHousingCommand extends StatsCommand {
     protected int getHdbCount(List<Person> fosterers) {
         return (int) fosterers.stream()
                 .filter(fosterer ->
-                        fosterer.getHousing().equals(Housing.HDB))
+                        fosterer.getHousing()
+                                .equals(Housing.HDB))
                 .count();
     }
 
     /**
      * Returns the number of fosterers who stay in Condos.
      */
     protected int getCondoCount(List<Person> fosterers) {
-        Housing condo = new Housing("Condo");
         return (int) fosterers.stream()
                 .filter(fosterer ->
-                        fosterer.getHousing().equals(Housing.CONDO))
+                        fosterer.getHousing()
+                                .equals(Housing.CONDO))
                 .count();
     }
 
     /**
      * Returns the number of fosterers who stay in Landed.
      */
     protected int getLandedCount(List<Person> fosterers) {
-        Housing landed = new Housing("Landed");
         return (int) fosterers.stream()
                 .filter(fosterer ->
-                        fosterer.getHousing().equals(Housing.LANDED))
+                        fosterer.getHousing()
+                                .equals(Housing.LANDED))
                 .count();
     }
 

src/main/java/seedu/address/model/person/Person.java

@@ -152,7 +152,7 @@ public void setNote(String note) {
      * Intended for use with predicates generated through the find command.
      *
      * @return a Map; keys include all publicly gettable fields as well as all tags,
-     *         and values are values of the respective fields, or {@code null} for tags.
+     *     and values are values of the respective fields, or {@code null} for tags.
      */
     public Map<String, String> getFieldsAndAttributes() {
         HashMap<String, String> map = new HashMap<>();
@@ -169,7 +169,7 @@ public Map<String, String> getFieldsAndAttributes() {
         return map;
     }
 
-    private void tryPut(Map<String, String> map, String key, Object value) {
+    protected void tryPut(Map<String, String> map, String key, Object value) {
         if (Objects.isNull(value)) {
             return;
         }
@@ -185,6 +185,27 @@ public Set<Tag> getTags() {
     }
 
 
+    /**
+     * Returns true if person is an available fosterer.
+     */
+    public boolean isAvailableFosterer() {
+        return availability.equals(Availability.AVAILABLE);
+    }
+
+    /**
+     * Returns true if person is a current fosterer.
+     * Person is a current fosterer if and only if they are not available, and is
+     * currently fostering an animal.
+     */
+    public boolean isCurrentFosterer() {
+        boolean isNotAvailable = this.availability.equals(Availability.NOT_AVAILABLE);
+        boolean isAnimalNameNil = this.animalName.fullName.equals(Person.NIL_WORD);
+        boolean isAnimalCurrentCatOrDog = this.animalType.equals(AnimalType.CURRENT_DOG)
+                || this.animalType.equals(AnimalType.CURRENT_CAT);
+
+        return isNotAvailable && !isAnimalNameNil && isAnimalCurrentCatOrDog;
+    }
+
     /**
      * Returns true if both persons have the same name.
      * This defines a weaker notion of equality between two persons.

src/test/java/seedu/address/logic/commands/DeleteCommandTest.java

@@ -40,8 +40,10 @@ public class DeleteCommandTest {
     @Test
     public void isValidIndex() throws CommandException {
         assertTrue(DeleteCommand.isValidIndex(0, 1));
-        assertThrows(CommandException.class, () -> DeleteCommand.isValidIndex(1, 1));
+        assertTrue(DeleteCommand.isValidIndex(0, 2));
         assertThrows(CommandException.class, () -> DeleteCommand.isValidIndex(-1, 1));
+        assertThrows(CommandException.class, () -> DeleteCommand.isValidIndex(1, -1));
+        assertThrows(CommandException.class, () -> DeleteCommand.isValidIndex(1, 1));
     }
 
     @Test

src/test/java/seedu/address/logic/commands/StatsAvailCommandTest.java

@@ -122,7 +122,6 @@ public void execute_noFosterers_throwsCommandException() {
 
     @Test
     public void execute_filteredList_expectZero() throws CommandException {
-        //should
         model.updateFilteredPersonList(fosterer -> !TypicalPersons.getAvailableFosterers().contains(fosterer));
         List<Person> fosterers = model.getFilteredPersonList();
         int total = fosterers.size();

src/test/java/seedu/address/logic/commands/StatsCommandTest.java

@@ -8,10 +8,14 @@
 public class StatsCommandTest {
 
     @Test
-    public void calculatePercentage() {
+    public void calculatePercentage_demomZero() {
         assertThrows(AssertionError.class, () -> {
             StatsCommand.calculatePercentage(1, 0);
         });
+    }
+
+    @Test
+    public void calculatePercentage() {
 
         assertEquals(0.0, StatsCommand.calculatePercentage(0, 10));
         assertEquals(3 / (double) 10 * 100, StatsCommand.calculatePercentage(3, 10));