Merge pull request #307 from TehOPanas/CodeQuality

Improve code quality for Insurance feature

src/main/java/seedu/address/logic/Messages.java

@@ -1,6 +1,6 @@
 package seedu.address.logic;
 
-import static seedu.address.model.insurance.Insurance.MAX_INSURANCE_COUNT;
+import static seedu.address.model.person.Insurance.MAX_INSURANCE_COUNT;
 import static seedu.address.model.person.Tag.MAXIMUM_TAGS_PER_PERSON;
 
 import java.util.Set;

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

@@ -6,7 +6,7 @@
 import static seedu.address.logic.Messages.MESSAGE_INSURANCE_COUNT_EXCEED;
 import static seedu.address.logic.Messages.MESSAGE_PERSON_NOT_CHANGED;
 import static seedu.address.logic.Messages.MESSAGE_TAG_COUNT_EXCEED;
-import static seedu.address.model.insurance.Insurance.MAX_INSURANCE_COUNT;
+import static seedu.address.model.person.Insurance.MAX_INSURANCE_COUNT;
 import static seedu.address.model.person.Tag.MAXIMUM_TAGS_PER_PERSON;
 
 import java.util.List;

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

@@ -1,14 +1,14 @@
 package seedu.address.logic.commands;
 
 import static seedu.address.commons.util.CollectionUtil.requireAllNonNull;
-import static seedu.address.logic.Messages.MESSAGE_INSURANCE_COUNT_EXCEED;
+import static seedu.address.logic.commands.CommandUtil.getPersonAtIndex;
+import static seedu.address.logic.commands.CommandUtil.verifyPersonChanged;
+import static seedu.address.logic.commands.CommandUtil.verifyPersonInsuranceCountIsValid;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_ADD_INSURANCE;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_DELETE_INSURANCE;
-import static seedu.address.model.insurance.Insurance.MAX_INSURANCE_COUNT;
 import static seedu.address.model.person.Person.createPersonWithUpdatedInsurances;
 
 import java.util.HashSet;
-import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Logger;
@@ -18,7 +18,7 @@
 import seedu.address.logic.Messages;
 import seedu.address.logic.commands.exceptions.CommandException;
 import seedu.address.model.Model;
-import seedu.address.model.insurance.Insurance;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Person;
 
 /**
@@ -45,12 +45,12 @@ public class InsuranceCommand extends Command {
 
     private static final Logger logger = LogsCenter.getLogger(InsuranceCommand.class);
 
-    private Index index;
+    private final Index index;
 
-    private UpdatePersonInsuranceDescriptor updatePersonInsuranceDescriptor;
+    private final UpdatePersonInsuranceDescriptor updatePersonInsuranceDescriptor;
 
     /**
-     * Instantiate {@code InsuranceCommmand}
+     * Instantiate {@code InsuranceCommand}
      */
     public InsuranceCommand(Index i, UpdatePersonInsuranceDescriptor u) {
         requireAllNonNull(i, u);
@@ -70,33 +70,37 @@ public InsuranceCommand(Index i, UpdatePersonInsuranceDescriptor u) {
     @Override
     public CommandResult execute(Model m) throws CommandException {
         requireAllNonNull(m);
-        List<Person> personList = m.getFilteredPersonList();
+        verifyNoConflictingInsurance();
 
-        if (index.getZeroBased() >= personList.size()) {
-            logger.finer(String.format("InsuranceCommand execution failed due to index %d out of bound",
-                    index.getOneBased()));
-            throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX);
-        }
-
-        assert index.getZeroBased() < personList.size() : "index < listSize";
+        return updatePerson(m);
+    }
 
+    /**
+     * Check for conflicting {@code Insurance} for the given command
+     *
+     * @throws CommandException when command add and delete the same {@code Insurance} at the same time
+     */
+    private void verifyNoConflictingInsurance() throws CommandException {
         if (updatePersonInsuranceDescriptor.hasCommonInsurance()) {
             throw new CommandException(MESSAGE_INSURANCE_CONFLICT);
         }
+    }
 
-        Person personToUpdate = personList.get(index.getZeroBased());
+    /**
+     * Update the customer details in the contact book
+     *
+     * @param m {@code Model}
+     * @return {@code CommandResult} result message of the command execution
+     * @throws CommandException when the execution of command is not allowed
+     */
+    private CommandResult updatePerson(Model m) throws CommandException {
+        Person personToUpdate = getPersonAtIndex(m, index);
         Person updatedPerson = createPersonWithUpdatedInsurances(personToUpdate,
                 updatePersonInsuranceDescriptor.insurancesToAdd,
                 updatePersonInsuranceDescriptor.insurancesToDelete);
 
-        if (updatedPerson.getInsurancesCount() > MAX_INSURANCE_COUNT) {
-            logger.finer("InsuranceCommand execution failed due to exceeding maximum insurance counts allowed");
-            throw new CommandException(MESSAGE_INSURANCE_COUNT_EXCEED);
-        }
-
-        requireAllNonNull(personToUpdate, updatedPerson);
-        CommandUtil.verifyPersonChanged(personToUpdate, updatedPerson,
-                Optional.of(MESSAGE_INSURANCE_UNCHANGED_REASONS));
+        verifyPersonInsuranceCountIsValid(updatedPerson);
+        verifyPersonChanged(personToUpdate, updatedPerson, Optional.of(MESSAGE_INSURANCE_UNCHANGED_REASONS));
 
         m.setPerson(personToUpdate, updatedPerson);
 

src/main/java/seedu/address/logic/parser/AddCommandParser.java

@@ -15,11 +15,11 @@
 
 import seedu.address.logic.commands.AddCommand;
 import seedu.address.logic.parser.exceptions.ParseException;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.person.Address;
 import seedu.address.model.person.Appointment;
 import seedu.address.model.person.AppointmentCount;
 import seedu.address.model.person.Email;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Name;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Phone;

src/main/java/seedu/address/logic/parser/InsuranceCommandParser.java

@@ -14,7 +14,7 @@
 import seedu.address.logic.commands.InsuranceCommand;
 import seedu.address.logic.commands.InsuranceCommand.UpdatePersonInsuranceDescriptor;
 import seedu.address.logic.parser.exceptions.ParseException;
-import seedu.address.model.insurance.Insurance;
+import seedu.address.model.person.Insurance;
 
 
 /**
@@ -23,6 +23,8 @@
  */
 public class InsuranceCommandParser implements Parser<InsuranceCommand> {
 
+    private ArgumentMultimap argMultimap;
+
     /**
      * Parse the given arguments and extract out the useful information for InsuranceCommand
      *
@@ -32,21 +34,37 @@ public class InsuranceCommandParser implements Parser<InsuranceCommand> {
      */
     @Override
     public InsuranceCommand parse(String args) throws ParseException {
-        Index index = null;
-
         requireAllNonNull(args);
 
-        ArgumentMultimap argMultimap =
-                ArgumentTokenizer.tokenize(args, PREFIX_ADD_INSURANCE, PREFIX_DELETE_INSURANCE);
+        argMultimap = ArgumentTokenizer.tokenize(args, PREFIX_ADD_INSURANCE, PREFIX_DELETE_INSURANCE);
+
+        Index index = obtainIndex();
+        UpdatePersonInsuranceDescriptor changes = obtainChanges();
+
+        return new InsuranceCommand(index, changes);
+    }
 
+    /**
+     * Check the parsed arguments for the {@code Index}
+     *
+     * @return {@code Index} indicated by the command if valid
+     * @throws ParseException when {@code Index} is out of bound or invalid values is passed in
+     */
+    private Index obtainIndex() throws ParseException {
         try {
-            index = ParserUtil.parseIndex(argMultimap.getPreamble());
+            return ParserUtil.parseIndex(argMultimap.getPreamble());
         } catch (ParseException e) {
             throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, InsuranceCommand.MESSAGE_USAGE), e);
         }
+    }
 
-        requireAllNonNull(index);
-
+    /**
+     * Check the parsed arguments for {@code Insurance} to update
+     *
+     * @return {@code UpdatePersonInsuranceDescriptor} that holds the {@code Insurance} to update on {@code Person}
+     * @throws ParseException when there is invalid or conflicting {@code Insurance}
+     */
+    private UpdatePersonInsuranceDescriptor obtainChanges() throws ParseException {
         Set<Insurance> insurancesToAdd =
                 parseInsurances(argMultimap.getAllValues(PREFIX_ADD_INSURANCE)).orElse(new HashSet<>());
 
@@ -62,7 +80,7 @@ public InsuranceCommand parse(String args) throws ParseException {
             throw new ParseException(InsuranceCommand.MESSAGE_INSURANCE_NO_UPDATE);
         }
 
-        return new InsuranceCommand(index, changes);
+        return changes;
     }
 
     /**

src/main/java/seedu/address/logic/parser/ParserUtil.java

@@ -16,10 +16,10 @@
 import seedu.address.commons.core.index.Index;
 import seedu.address.commons.util.StringUtil;
 import seedu.address.logic.parser.exceptions.ParseException;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.person.Address;
 import seedu.address.model.person.Appointment;
 import seedu.address.model.person.Email;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Name;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Phone;
@@ -142,7 +142,7 @@ public static Set<Tag> parseTags(Collection<String> tags) throws ParseException
 
     /**
      * Parse a {@code String insurance} into a {@code Insurance}
-     * Leading and trailing whitespaces will be trimmed.
+     * Leading, trailing and contiguous whitespaces between words will be trimmed.
      *
      */
     private static Insurance parseInsurance(String insurance) throws ParseException {

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

@@ -32,8 +32,8 @@ public static Address createAddress(String address) {
     public static boolean isValidAddress(String test) {
         requireNonNull(test);
 
-        int addressLen = test.trim().length();
-        return addressLen <= MAX_LENGTH;
+        int addressLength = test.trim().length();
+        return addressLength <= MAX_LENGTH;
     }
 
 

src/main/java/seedu/address/model/insurance/Insurance.java → src/main/java/seedu/address/model/person/Insurance.java

@@ -1,4 +1,4 @@
-package seedu.address.model.insurance;
+package seedu.address.model.person;
 
 import static java.util.Objects.requireNonNull;
 import static seedu.address.commons.util.AppUtil.checkArgument;
@@ -17,7 +17,7 @@ public class Insurance {
     public static final String MESSAGE_CONSTRAINTS =
             String.format("Insurance name should be alphanumeric,"
                     + "non-empty and not longer than %d characters", MAX_LENGTH);
-    private String insuranceName;
+    private final String insuranceName;
 
     /**
      * Constructs an {@code Insurance}.

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

@@ -10,7 +10,6 @@
 
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.commands.EditCommand.EditPersonDescriptor;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.priority.Priority;
 import seedu.address.model.priority.Priority.Level;
 

src/main/java/seedu/address/model/person/predicate/InsuranceContainsKeywordsPredicate.java

@@ -5,7 +5,7 @@
 import java.util.function.Predicate;
 
 import seedu.address.commons.util.ToStringBuilder;
-import seedu.address.model.insurance.Insurance;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Person;
 
 /**

src/main/java/seedu/address/model/util/SampleDataUtil.java

@@ -8,10 +8,10 @@
 
 import seedu.address.model.AddressBook;
 import seedu.address.model.ReadOnlyAddressBook;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.person.Appointment;
 import seedu.address.model.person.AppointmentCount;
 import seedu.address.model.person.Email;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Name;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Phone;

src/main/java/seedu/address/storage/jsonadaptedperson/JsonAdaptedInsurance.java

@@ -6,8 +6,8 @@
 import com.fasterxml.jackson.annotation.JsonValue;
 
 import seedu.address.commons.exceptions.IllegalValueException;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.person.Email;
+import seedu.address.model.person.Insurance;
 
 /**
  * Jackson-friendly version of {@code Insurance}

src/main/java/seedu/address/storage/jsonadaptedperson/JsonAdaptedPerson.java

@@ -12,11 +12,11 @@
 import com.fasterxml.jackson.annotation.JsonProperty;
 
 import seedu.address.commons.exceptions.IllegalValueException;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.person.Address;
 import seedu.address.model.person.Appointment;
 import seedu.address.model.person.AppointmentCount;
 import seedu.address.model.person.Email;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Name;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Phone;

src/main/java/seedu/address/ui/PersonCard.java

@@ -12,7 +12,7 @@
 import javafx.scene.layout.HBox;
 import javafx.scene.layout.Region;
 import javafx.scene.layout.VBox;
-import seedu.address.model.insurance.Insurance;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Tag;
 import seedu.address.model.priority.Priority;

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

@@ -25,8 +25,8 @@
 import seedu.address.model.Model;
 import seedu.address.model.ModelManager;
 import seedu.address.model.UserPrefs;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.person.Appointment;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Remark;
 import seedu.address.model.person.Tag;

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

@@ -23,7 +23,7 @@
 import seedu.address.model.Model;
 import seedu.address.model.ModelManager;
 import seedu.address.model.UserPrefs;
-import seedu.address.model.insurance.Insurance;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Person;
 import seedu.address.testutil.PersonBuilder;
 
@@ -184,7 +184,6 @@ public void execute_insuranceCountAtLimit_success() {
 
     }
 
-
     @Test
     public void execute_conflictingAddDelete_throwsCommandException() {
         defaultDescriptor.setInsurancesToAdd(new Insurance(healthInsurance));

src/test/java/seedu/address/logic/parser/AddCommandParserTest.java

@@ -62,10 +62,10 @@
 
 import seedu.address.logic.Messages;
 import seedu.address.logic.commands.AddCommand;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.person.Address;
 import seedu.address.model.person.Appointment;
 import seedu.address.model.person.Email;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Name;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Phone;

src/test/java/seedu/address/logic/parser/AddressBookParserTest.java

@@ -34,8 +34,8 @@
 import seedu.address.logic.commands.TagCommand;
 import seedu.address.logic.commands.TagCommand.UpdatePersonTagsDescriptor;
 import seedu.address.logic.parser.exceptions.ParseException;
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.person.Appointment;
+import seedu.address.model.person.Insurance;
 import seedu.address.model.person.Person;
 import seedu.address.model.person.Tag;
 import seedu.address.model.person.predicate.NameContainsKeywordsPredicate;

src/test/java/seedu/address/logic/parser/InsuranceCommandParserTest.java

@@ -14,7 +14,7 @@
 import seedu.address.commons.core.index.Index;
 import seedu.address.logic.commands.InsuranceCommand;
 import seedu.address.logic.commands.InsuranceCommand.UpdatePersonInsuranceDescriptor;
-import seedu.address.model.insurance.Insurance;
+import seedu.address.model.person.Insurance;
 
 public class InsuranceCommandParserTest {
 
@@ -33,7 +33,7 @@ public void parse_nullArgument_throwsNullPointerException() {
     }
 
     @Test
-    public void parse_addOneInsurnance_success() {
+    public void parse_addOneInsurance_success() {
         descriptor.setInsurancesToAdd(new Insurance("car insurance"));
 
         String argument = "1 ai/car insurance";

src/test/java/seedu/address/model/person/InsuranceTest.java

@@ -2,13 +2,11 @@
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
-import static seedu.address.model.insurance.Insurance.isValidInsuranceName;
+import static seedu.address.model.person.Insurance.isValidInsuranceName;
 import static seedu.address.testutil.Assert.assertThrows;
 
 import org.junit.jupiter.api.Test;
 
-import seedu.address.model.insurance.Insurance;
-
 public class InsuranceTest {
 
     private String invalidArgument = "  ";

src/test/java/seedu/address/model/person/PersonTest.java

@@ -34,7 +34,6 @@
 
 import org.junit.jupiter.api.Test;
 
-import seedu.address.model.insurance.Insurance;
 import seedu.address.model.priority.Priority;
 import seedu.address.testutil.PersonBuilder;