Add code quality improvements

AY2122S1-CS2103-W14-1 · Nov 6, 2021 · deec35f · deec35f
1 parent c581369
commit deec35f
Show file tree

Hide file tree

Showing 7 changed files with 46 additions and 41 deletions.
diff --git a/src/main/java/seedu/docit/logic/commands/prescription/AddPrescriptionCommand.java b/src/main/java/seedu/docit/logic/commands/prescription/AddPrescriptionCommand.java
@@ -41,9 +41,15 @@ public class AddPrescriptionCommand extends AppointmentCommand {
             + "Volume: %2$s\nDuration: %3$s";
     public static final String MESSAGE_DUPLICATE_MEDICINE =
             "This medicine already exists in the prescription for this appointment";
-    public static final String MESSAGE_FIELD_TOO_LONG =
+    private static final String MESSAGE_FIELD_TOO_LONG =
             "Medicine name can only be %1$s characters long. \nVolume field can only be %2$s characters long. "
                     + "\nDuration field can only be %3$s characters long.";
+
+    public static final String INPUT_TOO_LONG_ERROR_MESSAGE = String.format(MESSAGE_FIELD_TOO_LONG,
+            Prescription.MEDICINE_CHAR_LENGTH_LIMIT,
+            Prescription.VOLUME_CHAR_LENGTH_LIMIT,
+            Prescription.DURATION_CHAR_LENGTH_LIMIT);
+
     private static Logger logger = Logger.getLogger("AddPrescriptionCommand");
 
     private final Index targetAppointmentIndex;
@@ -77,6 +83,8 @@ public CommandResult execute(Model model) throws CommandException {
                     + Messages.MESSAGE_INVALID_APPOINTMENT_DISPLAYED_INDEX);
             throw new CommandException(Messages.MESSAGE_INVALID_APPOINTMENT_DISPLAYED_INDEX);
         }
+        assert (targetAppointmentIndex.getZeroBased() >= 0
+                && targetAppointmentIndex.getZeroBased() < lastShownList.size());
         Appointment appointmentToMakePrescription = lastShownList.get(targetAppointmentIndex.getZeroBased());
         Prescription prescriptionToAdd = new Prescription(medicine, volume, duration);
 
@@ -89,16 +97,8 @@ public CommandResult execute(Model model) throws CommandException {
         if (volume.length() > Prescription.VOLUME_CHAR_LENGTH_LIMIT
                 || medicine.length() > Prescription.MEDICINE_CHAR_LENGTH_LIMIT
                 || duration.length() > Prescription.DURATION_CHAR_LENGTH_LIMIT) {
-            logger.log(Level.WARNING, String.format(MESSAGE_FIELD_TOO_LONG,
-                    Prescription.MEDICINE_CHAR_LENGTH_LIMIT,
-                    Prescription.VOLUME_CHAR_LENGTH_LIMIT,
-                    Prescription.DURATION_CHAR_LENGTH_LIMIT
-            ));
-            throw new CommandException(String.format(MESSAGE_FIELD_TOO_LONG,
-                    Prescription.MEDICINE_CHAR_LENGTH_LIMIT,
-                    Prescription.VOLUME_CHAR_LENGTH_LIMIT,
-                    Prescription.DURATION_CHAR_LENGTH_LIMIT
-            ));
+            logger.log(Level.WARNING, INPUT_TOO_LONG_ERROR_MESSAGE);
+            throw new CommandException(INPUT_TOO_LONG_ERROR_MESSAGE);
         }
 
         model.addPrescription(appointmentToMakePrescription, prescriptionToAdd);

diff --git a/src/main/java/seedu/docit/logic/commands/prescription/DeletePrescriptionCommand.java b/src/main/java/seedu/docit/logic/commands/prescription/DeletePrescriptionCommand.java
@@ -4,6 +4,8 @@
 
 import java.util.List;
 import java.util.Objects;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 
 import seedu.docit.commons.core.Messages;
 import seedu.docit.commons.core.index.Index;
@@ -32,6 +34,9 @@ public class DeletePrescriptionCommand extends AppointmentCommand {
     public static final String MESSAGE_DELETE_PRESCRIPTION_SUCCESS = "Deleted prescription: \nMedicine: %1$s\n\n"
             + "from %2$s's appointment.";
 
+    private static Logger logger = Logger.getLogger("DeletePrescriptionCommand");
+
+
     private final Index targetAppointmentIndex;
     private final String targetMedicineName;
 
@@ -46,19 +51,27 @@ public DeletePrescriptionCommand(Index targetAppointmentIndex, String targetMedi
     }
 
     @Override public CommandResult execute(Model model) throws CommandException {
+        logger.log(Level.INFO, "going to start deleting prescription");
         requireNonNull(model);
         List<Appointment> lastShownList = model.getFilteredAppointmentList();
 
         if (targetAppointmentIndex.getZeroBased() >= lastShownList.size()) {
+            logger.log(Level.WARNING, "deleting prescription failed, "
+                    + Messages.MESSAGE_INVALID_APPOINTMENT_DISPLAYED_INDEX);
             throw new CommandException(Messages.MESSAGE_INVALID_APPOINTMENT_DISPLAYED_INDEX);
         }
+        assert (targetAppointmentIndex.getZeroBased() >= 0
+                && targetAppointmentIndex.getZeroBased() < lastShownList.size());
 
         Appointment appointmentToTarget = lastShownList.get(targetAppointmentIndex.getZeroBased());
         try {
             model.deletePrescription(appointmentToTarget, targetMedicineName);
+            logger.log(Level.INFO, "deleting prescription success");
             return new CommandResult(String.format(MESSAGE_DELETE_PRESCRIPTION_SUCCESS,
                     targetMedicineName, appointmentToTarget.getPatient().getName()));
         } catch (MedicineNotFoundException e) {
+            logger.log(Level.WARNING, "deleting prescription failed, "
+                    + e.getMessage());
             throw new CommandException(e.getMessage());
         }
     }

diff --git a/src/main/java/seedu/docit/logic/parser/prescription/AddPrescriptionCommandParser.java b/src/main/java/seedu/docit/logic/parser/prescription/AddPrescriptionCommandParser.java
@@ -16,6 +16,7 @@
  * Parses input arguments and creates a new AddPrescriptionCommand object
  */
 public class AddPrescriptionCommandParser implements AppointmentParser<AddPrescriptionCommand> {
+    public static final String EMPTY_FIELD_ERROR_MESSAGE = "Medicine/Duration/Volume fields cannot be blank.";
 
     /**
      * Parses the given {@code String} of arguments in the context of the AddPrescriptionCommand and returns an
@@ -41,7 +42,7 @@ public AddPrescriptionCommand parseAppointmentCommand(String args) throws ParseE
         String volume = argMultimap.getValue(CliSyntax.PREFIX_VOLUME).get();
 
         if (medicineName.isBlank() || duration.isBlank() || volume.isBlank()) {
-            throw new ParseException("Medicine/Duration/Volume fields cannot be blank.");
+            throw new ParseException(EMPTY_FIELD_ERROR_MESSAGE);
         }
         return new AddPrescriptionCommand(appointmentIndex, medicineName, volume, duration);
     }

diff --git a/src/main/java/seedu/docit/logic/parser/prescription/DeletePrescriptionCommandParser.java b/src/main/java/seedu/docit/logic/parser/prescription/DeletePrescriptionCommandParser.java
@@ -12,6 +12,7 @@
 import seedu.docit.logic.parser.exceptions.ParseException;
 
 public class DeletePrescriptionCommandParser implements AppointmentParser<DeletePrescriptionCommand> {
+    public static final String EMPTY_MEDICINE_FIELD_ERROR_MESSAGE = "Medicine fields cannot be blank.";
     /**
      * Parses the given {@code String} of arguments in the context of the DeletePrescriptionCommand and returns a
      * DeletePrescriptionCommand object for execution.
@@ -37,7 +38,7 @@ public DeletePrescriptionCommand parseAppointmentCommand(String args) throws Par
         String medicineName = argMultimap.getValue(CliSyntax.PREFIX_NAME).get();
 
         if (medicineName.isBlank()) {
-            throw new ParseException("Medicine fields cannot be blank.");
+            throw new ParseException(EMPTY_MEDICINE_FIELD_ERROR_MESSAGE);
         }
         return new DeletePrescriptionCommand(index, medicineName);
     }

diff --git a/src/test/java/seedu/docit/logic/commands/prescriptions/AddPrescriptionCommandTest.java b/src/test/java/seedu/docit/logic/commands/prescriptions/AddPrescriptionCommandTest.java
@@ -3,7 +3,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static seedu.docit.commons.core.Messages.MESSAGE_INVALID_APPOINTMENT_DISPLAYED_INDEX;
-import static seedu.docit.logic.commands.prescription.AddPrescriptionCommand.MESSAGE_FIELD_TOO_LONG;
+import static seedu.docit.logic.commands.prescription.AddPrescriptionCommand.INPUT_TOO_LONG_ERROR_MESSAGE;
 import static seedu.docit.logic.commands.prescription.AddPrescriptionCommand.MESSAGE_SUCCESS;
 import static seedu.docit.testutil.Assert.assertThrows;
 import static seedu.docit.testutil.TypicalAppointments.getTypicalAppointmentList;
@@ -91,11 +91,7 @@ public void execute_appointmentToAddMedicineTooLong_throwsCommandException() {
         AddPrescriptionCommand invalidAddPrescriptionCommand =
                 new AddPrescriptionCommand(Index.fromOneBased(1), longMedicineName, defaultVolume, defaultDuration);
 
-        assertThrows(CommandException.class, String.format(MESSAGE_FIELD_TOO_LONG,
-                Prescription.MEDICINE_CHAR_LENGTH_LIMIT,
-                Prescription.VOLUME_CHAR_LENGTH_LIMIT,
-                Prescription.DURATION_CHAR_LENGTH_LIMIT
-        ), () ->
+        assertThrows(CommandException.class, INPUT_TOO_LONG_ERROR_MESSAGE, () ->
                 invalidAddPrescriptionCommand.execute(model));
 
     }
@@ -106,11 +102,7 @@ public void execute_appointmentToAddVolumeTooLong_throwsCommandException() {
         AddPrescriptionCommand invalidAddPrescriptionCommand =
                 new AddPrescriptionCommand(Index.fromOneBased(3), defaultMedicine, longVolumeInput, defaultDuration);
 
-        assertThrows(CommandException.class, String.format(MESSAGE_FIELD_TOO_LONG,
-                Prescription.MEDICINE_CHAR_LENGTH_LIMIT,
-                Prescription.VOLUME_CHAR_LENGTH_LIMIT,
-                Prescription.DURATION_CHAR_LENGTH_LIMIT
-        ), () ->
+        assertThrows(CommandException.class, INPUT_TOO_LONG_ERROR_MESSAGE, () ->
                 invalidAddPrescriptionCommand.execute(model));
 
     }
@@ -121,11 +113,7 @@ public void execute_appointmentToAddDurationTooLong_throwsCommandException() {
         AddPrescriptionCommand invalidAddPrescriptionCommand =
                 new AddPrescriptionCommand(Index.fromOneBased(1), defaultMedicine, defaultVolume, longDurationInput);
 
-        assertThrows(CommandException.class, String.format(MESSAGE_FIELD_TOO_LONG,
-                Prescription.MEDICINE_CHAR_LENGTH_LIMIT,
-                Prescription.VOLUME_CHAR_LENGTH_LIMIT,
-                Prescription.DURATION_CHAR_LENGTH_LIMIT
-        ), () ->
+        assertThrows(CommandException.class, INPUT_TOO_LONG_ERROR_MESSAGE, () ->
                 invalidAddPrescriptionCommand.execute(model));
 
     }

diff --git a/src/test/java/seedu/docit/logic/parser/prescription/AddPrescriptionCommandParserTest.java b/src/test/java/seedu/docit/logic/parser/prescription/AddPrescriptionCommandParserTest.java
@@ -10,6 +10,7 @@
 import static seedu.docit.logic.parser.CliSyntax.PREFIX_DURATION;
 import static seedu.docit.logic.parser.CliSyntax.PREFIX_NAME;
 import static seedu.docit.logic.parser.CliSyntax.PREFIX_VOLUME;
+import static seedu.docit.logic.parser.prescription.AddPrescriptionCommandParser.EMPTY_FIELD_ERROR_MESSAGE;
 
 import org.junit.jupiter.api.Test;
 
@@ -22,7 +23,7 @@ public class AddPrescriptionCommandParserTest {
     private final AddPrescriptionCommandParser parser = new AddPrescriptionCommandParser();
 
     @Test
-    public void parse_allFieldsPresent_success() throws ParseException {
+    public void parseAppointmentCommand_allFieldsPresent_success() throws ParseException {
         assertParseSuccess(parser, VALID_APPOINTMENT_INDEX + " "
                         + PREFIX_NAME + VALID_PRESCRIPTION_MEDICINE + " "
                         + PREFIX_VOLUME + VALID_PRESCRIPTION_VOLUME + " "
@@ -34,7 +35,7 @@ public void parse_allFieldsPresent_success() throws ParseException {
     }
 
     @Test
-    public void parse_allFieldsBlank_failure() throws ParseException {
+    public void parseAppointmentCommand_allFieldsBlank_failure() throws ParseException {
         assertParseFailure(parser, " "
                         + PREFIX_NAME + " "
                         + PREFIX_VOLUME + " "
@@ -43,15 +44,15 @@ public void parse_allFieldsBlank_failure() throws ParseException {
     }
 
     @Test
-    public void parse_noIndex_failure() throws ParseException {
+    public void parseAppointmentCommand_noIndex_failure() throws ParseException {
         assertParseFailure(parser, PREFIX_NAME + VALID_PRESCRIPTION_MEDICINE + " "
                         + PREFIX_VOLUME + VALID_PRESCRIPTION_VOLUME + " "
                         + PREFIX_DURATION + VALID_PRESCRIPTION_DURATION,
                 String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddPrescriptionCommand.MESSAGE_USAGE));
     }
 
     @Test
-    public void parse_noPrefix_failure() throws ParseException {
+    public void parseAppointmentCommand_noPrefix_failure() throws ParseException {
         assertParseFailure(parser, VALID_APPOINTMENT_INDEX + " "
                         + VALID_PRESCRIPTION_MEDICINE + " "
                         + VALID_PRESCRIPTION_VOLUME + " "
@@ -60,12 +61,12 @@ public void parse_noPrefix_failure() throws ParseException {
     }
 
     @Test
-    public void parse_blankEntry_failure() throws ParseException {
+    public void parseAppointmentCommand_blankEntry_failure() throws ParseException {
         assertParseFailure(parser, VALID_APPOINTMENT_INDEX + " "
                         + PREFIX_NAME + " "
                         + PREFIX_VOLUME + " "
                         + PREFIX_DURATION,
-                "Medicine/Duration/Volume fields cannot be blank.");
+                EMPTY_FIELD_ERROR_MESSAGE);
     }
 
 }
diff --git a/src/test/java/seedu/docit/logic/parser/prescription/DeletePrescriptionCommandParserTest.java b/src/test/java/seedu/docit/logic/parser/prescription/DeletePrescriptionCommandParserTest.java
@@ -6,6 +6,7 @@
 import static seedu.docit.logic.parser.AppointmentCommandParserTestUtil.assertParseFailure;
 import static seedu.docit.logic.parser.AppointmentCommandParserTestUtil.assertParseSuccess;
 import static seedu.docit.logic.parser.CliSyntax.PREFIX_NAME;
+import static seedu.docit.logic.parser.prescription.DeletePrescriptionCommandParser.EMPTY_MEDICINE_FIELD_ERROR_MESSAGE;
 import static seedu.docit.testutil.TypicalAppointments.getTypicalAppointmentList;
 import static seedu.docit.testutil.TypicalPatients.getTypicalAddressBook;
 
@@ -37,38 +38,38 @@ public class DeletePrescriptionCommandParserTest {
     private final DeletePrescriptionCommandParser parser = new DeletePrescriptionCommandParser();
 
     @Test
-    public void parse_allFieldsPresent_success() throws ParseException {
+    public void parseAppointmentCommand_allFieldsPresent_success() throws ParseException {
         assertParseSuccess(parser, VALID_APPOINTMENT_INDEX + " "
                 + PREFIX_NAME + VALID_PRESCRIPTION_MEDICINE,
                 new DeletePrescriptionCommand(Index.fromOneBased(Integer.parseInt(VALID_APPOINTMENT_INDEX)),
                         VALID_PRESCRIPTION_MEDICINE));
     }
 
     @Test
-    public void parse_allFieldsBlank_failure() throws ParseException {
+    public void parseAppointmentCommand_allFieldsBlank_failure() throws ParseException {
         assertParseFailure(parser, " "
                         + PREFIX_NAME,
                 String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeletePrescriptionCommand.MESSAGE_USAGE));
     }
 
     @Test
-    public void parse_noIndex_failure() throws ParseException {
+    public void parseAppointmentCommand_noIndex_failure() throws ParseException {
         assertParseFailure(parser, " "
                         + PREFIX_NAME + VALID_PRESCRIPTION_MEDICINE,
                 String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeletePrescriptionCommand.MESSAGE_USAGE));
     }
 
     @Test
-    public void parse_badIndex_failure() throws ParseException {
+    public void parseAppointmentCommand_badIndex_failure() throws ParseException {
         assertParseFailure(parser, "notIndex "
                         + PREFIX_NAME + VALID_PRESCRIPTION_MEDICINE,
                 "Index is not a non-zero unsigned integer.");
     }
 
     @Test
-    public void parse_blankEntry_failure() throws ParseException {
+    public void parseAppointmentCommand_blankEntry_failure() throws ParseException {
         assertParseFailure(parser, VALID_APPOINTMENT_INDEX + " "
                         + PREFIX_NAME,
-                "Medicine fields cannot be blank.");
+                EMPTY_MEDICINE_FIELD_ERROR_MESSAGE);
     }
 }