Merge pull request AY2324S2-CS2103T-T15-1#242 from ararchch/branch-Co…

…deQuality

Code quality improvements

src/main/java/seedu/address/commons/util/IdUtil.java

@@ -9,12 +9,21 @@
 import jdk.jshell.spi.ExecutionControl;
 
 /**
- * Generates unique  String IDs for patients, doctors, and appointments
+ * Generates unique  String IDs for patients, doctors, and appointments.
+ *
+ * At the moment the ID util is underutilised, but we have kept it in the code for future adaptation.
+ * When the class was orignially defined, we envisioned it being essential to our use case, however, as development
+ * progressed, we realised that it would not add much significant value at least until v1.4 of our product.
+ * However, it can serve a purpose down the line, so we have left it in, despite it not being fleshed out completely.
+ *
+ * Currently only appointments are assigned an ID upon being created, but they ID itself does not serve a purpose.
  */
 public class IdUtil {
 
     /**
-     * Enum containing all possible entity types in our system
+     * Enum containing all possible entity types in our system.
+     *
+     * Associated characters are the first letter of each type of entity.
      */
     public enum Entities {
         PATIENT("p"),
@@ -27,15 +36,15 @@ public enum Entities {
         }
 
         /**
-         * Returns letter associated with entity
+         * Returns letter associated with entity.
          * @return String letter
          */
         public String getLetter() {
             return letter;
         }
 
         /**
-         * Gets entity object associated with character
+         * Gets entity object associated with character.
          *
          * @param c character in question
          * @return Entities entity object associated with input character
@@ -58,39 +67,46 @@ protected static Entities getEntityFromChar(char c) {
     /**
      * Generates a new id based on input entity.
      *
-     * @param entity type of id to generate
-     * @return String id
+     * @param entity type of id to generate.
+     * @return String id.
      */
     public static String generateNewId(Entities entity) {
         HashSet<String> idSet = allIds.get(entity);
         if (idSet == null) {
             idSet = new HashSet<>();
             allIds.put(entity, idSet);
         }
+
         Random random = new Random();
         String initId = String.valueOf(random.nextInt(90000000) + 10000000);
         while (idSet.contains(initId)) {
             initId = String.valueOf(random.nextInt(90000000) + 10000000);;
         }
+
         idSet.add(initId);
+        assert initId.length() == 8 : "All numeric portions of IDs must be 8 digits long";
 
         return entity.getLetter() + initId;
     }
 
     /**
-     * Deletes Id that is inputted
-     * @param id String id to delete
+     * Deletes Id that is inputted.
+     *
+     * @param id String id to delete.
      */
     public static void deleteId(String id) {
         requireNonNull(id);
-        Entities entity = Entities.getEntityFromChar(id.substring(0, 1).charAt(0));
+        char firstChar = id.substring(0, 1).charAt(0);
+        assert firstChar == 'a' || firstChar == 'p' || firstChar == 'd' : "IDs can only start with these 3 letters";
+        Entities entity = Entities.getEntityFromChar(firstChar);
         HashSet<String> idSet = allIds.get(entity);
         idSet.remove(id.substring(1, id.length()));
     }
 
     /**
-     * Returns allIds as an unmodifiable map
-     * @return unmodifiable map containing ids
+     * Returns allIds as an unmodifiable map.
+     *
+     * @return unmodifiable map containing ids.
      */
     public static boolean hasId(String id) {
         requireNonNull(id);
@@ -100,8 +116,10 @@ public static boolean hasId(String id) {
     }
 
     /**
-     * Updates map with initial values from storage
-     * @throws ExecutionControl.NotImplementedException
+     * Updates map with initial values from storage.
+     * To be implemented in the future as it does not affect or impact current functionality.
+     *
+     * @throws ExecutionControl.NotImplementedException as it is not implemented yet and shouldn't be used.
      */
     public static void initalMapUpdate() throws ExecutionControl.NotImplementedException {
         throw new ExecutionControl.NotImplementedException("to be implemented");

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

@@ -54,9 +54,10 @@ public static String format(Person person) {
     }
 
     /**
-     * Formats appointment for display in result box
-     * @param appointment the appointment in question
-     * @return String formatted string as per requirements
+     * Formats appointment for display in result box.
+     *
+     * @param appointment the appointment in question.
+     * @return String formatted string as per requirements.
      */
     public static String format(Appointment appointment) {
         final StringBuilder builder = new StringBuilder();

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

@@ -5,6 +5,10 @@
 import static seedu.address.logic.parser.CliSyntax.PREFIX_DOCTORNRIC;
 import static seedu.address.logic.parser.CliSyntax.PREFIX_PATIENTNRIC;
 
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import seedu.address.commons.core.LogsCenter;
 import seedu.address.commons.util.ToStringBuilder;
 import seedu.address.logic.Messages;
 import seedu.address.logic.commands.exceptions.CommandException;
@@ -13,12 +17,12 @@
 import seedu.address.model.appointment.exceptions.InvalidAppointmentException;
 import seedu.address.model.person.exceptions.PersonNotFoundException;
 
-
 /**
  * Command to add an appointment to MediCLI
  */
 public class AddAppointmentCommand extends Command {
 
+
     public static final String COMMAND_WORD = "addappt";
 
     public static final String MESSAGE_USAGE = COMMAND_WORD + ": Adds an appointment to the MediCLI system.\n"
@@ -34,35 +38,50 @@ public class AddAppointmentCommand extends Command {
     public static final String MESSAGE_SUCCESS = "New Appointment added: %1$s";
     public static final String MESSAGE_DUPLICATE_APPOINTMENT = "This appointment already exists in the MediCLI";
 
+    private static final Logger logger = LogsCenter.getLogger(AddAppointmentCommand.class);
+
     private final Appointment toAdd;
 
     /**
-     * Creates an AddCommand to add the specified {@code Person}
+     * Creates an AddCommand to add the specified {@code Appointment}
      */
     public AddAppointmentCommand(Appointment appointment) {
         requireNonNull(appointment);
         toAdd = appointment;
     }
 
+    /**
+     * Method that executes command when called by performing checks then adding to the list.
+     *
+     * @param model the model in which the command is executed
+     * @return CommandResult resulting from command execution
+     */
     @Override
     public CommandResult execute(Model model) throws CommandException {
         requireNonNull(model);
+        logger.log(Level.INFO, "going to add appointment");
 
         if (model.hasAppointment(toAdd)) {
+            logger.log(Level.INFO, "appointment was not added as it is in system");
             throw new CommandException(MESSAGE_DUPLICATE_APPOINTMENT);
         }
+
         try {
+            logger.log(Level.INFO, "checking if appointment is valid");
             if (!model.isValidAppointment(toAdd)) {
+                logger.log(Level.INFO, "appointment was not added as it is invalid");
                 throw new InvalidAppointmentException();
             }
         } catch (PersonNotFoundException e) {
             throw new CommandException("The provided Doctor / Patient is not registered in the system");
         }
 
         model.addAppointment(toAdd);
+        logger.log(Level.INFO, "appointment was added to system");
         return new CommandResult(String.format(MESSAGE_SUCCESS, Messages.format(toAdd)));
     }
 
+
     @Override
     public boolean equals(Object other) {
         if (other == this) {

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

@@ -83,8 +83,13 @@ public CommandResult execute(Model model) throws CommandException {
     }
 
     /**
-     * Creates and returns a {@code Appointment} with the details of {@code appointmentToEdit}
-     * edited with {@code editAppointmentDescriptor}.
+     * Creates and returns a {@code Appointment} with modified details of {@code appointmentToEdit}.
+     * Modified appointment is edited with {@code editAppointmentDescriptor}.
+     *
+     * @param appointmentToEdit the appointment to edit.
+     * @param editAppointmentDescriptor the descriptor to edit according to.
+     * @return Appointment with the details of appointmentToEdit.
+     * @throws CommandException if new inputs are invalid.
      */
     private static Appointment createEditedAppointment(
             Appointment appointmentToEdit,

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

@@ -14,14 +14,15 @@
 import seedu.address.model.person.Nric;
 
 /**
- * Parses addAppointment Command
+ * Parses addAppointment Command.
  */
 public class AddAppointmentCommandParser {
 
     /**
-     * Parses the given {@code String} of arguments in the context of the AddAppointmentCommand
-     * and returns an AddAppointmentCommand object for execution.
-     * @throws ParseException if the user input does not conform the expected format
+     * Parses the given {@code String} of arguments in the context of the AddAppointmentCommand.
+     * Returns an AddAppointmentCommand object for execution.
+     *
+     * @throws ParseException if the user input does not conform the expected format.
      */
     public AddAppointmentCommand parse(String args) throws ParseException {
         ArgumentMultimap argMultimap =
@@ -44,7 +45,7 @@ public AddAppointmentCommand parse(String args) throws ParseException {
     }
 
     /**
-     * Returns true if none of the prefixes contains empty {@code Optional} values in the given
+     * Returns true if none of the prefixes contains empty {@code Optional} values in the given.
      * {@code ArgumentMultimap}.
      */
     private static boolean arePrefixesPresent(ArgumentMultimap argumentMultimap, Prefix... prefixes) {

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

@@ -148,10 +148,11 @@ public static Email parseEmail(String email) throws ParseException {
     }
 
     /**
-     * Parses AppointmentDate from string to return an AppointmentDate object
-     * @param apptDateTime String to parse
-     * @return instance of AppointmentDate
-     * @throws ParseException if string is invalid date
+     * Parses AppointmentDateTime from string to return an AppointmentDateTime object.
+     *
+     * @param apptDateTime String to parse.
+     * @return instance of AppointmentDateTime.
+     * @throws ParseException if string is invalid date.
      */
     public static AppointmentDateTime parseAppointmentDateTime(String apptDateTime) throws ParseException {
         requireNonNull(apptDateTime);

src/main/java/seedu/address/model/AddressBook.java

@@ -64,6 +64,12 @@ public void setAppointments(List<Appointment> appointments) throws DuplicateAppo
         this.appointments.setAppointments(appointments);
     }
 
+    /**
+     * Replaces the contents of the appointment list with {@code appointments}.
+     * This method does not throw exception because it is only called when resetting data.
+     *
+     * @param appointments the appointments to update with no duplicates.
+     */
     public void setAppointmentsExistingBook(List<Appointment> appointments) {
         this.appointments.setAppointmentsExistingBook(appointments);
     }
@@ -88,20 +94,18 @@ public boolean hasPerson(Person person) {
         return persons.contains(person);
     }
 
+    /**
+     * Returns person (if any) with the provided NRIC in MediCLI.
+     *
+     * @param nricObj the NRIC to lookup.
+     * @return Person with corresponding NRIC.
+     * @throws PersonNotFoundException if no such Person exists.
+     */
     public Person getPersonByNric(Nric nricObj) throws PersonNotFoundException {
         requireNonNull(nricObj);
         return persons.getPersonByNric(nricObj);
     }
 
-    /**
-     * Returns true if a person with the same identity as {@code person} exists in the address book.
-     * @param nric input nric string
-     * @return boolean stating if addressbook contains nric
-     */
-    public boolean hasPersonNric(String nric) {
-        requireNonNull(nric);
-        return persons.containsNric(nric);
-    }
 
     /**
      * Adds a person to the address book.
@@ -147,14 +151,31 @@ public List<Appointment> getAppointmentByPerson(Person person) {
         return appointments.contains(person);
     }
 
+
+    /**
+     * Adds the specified appointment to the list of appointments.
+     *
+     * @param appointment The appointment to add to the list. It must not be null.
+     */
     public void addAppointment(Appointment appointment) {
         appointments.add(appointment);
     }
 
+    /**
+     * Removes the specified appointment from the list of appointments.
+     *
+     * @param appointment The appointment to be removed from the list. It must not be null.
+     */
     public void deleteAppointment(Appointment appointment) {
         appointments.remove(appointment);
     }
 
+    /**
+     * Checks if the specified appointment is present in the list of appointments.
+     *
+     * @param appointment The appointment to check in the list. It must not be null.
+     * @return true if the appointment is present in the list, false otherwise.
+     */
     public boolean hasAppointment(Appointment appointment) {
         return appointments.contains(appointment);
     }

src/main/java/seedu/address/model/Model.java

@@ -55,12 +55,12 @@ public interface Model {
     ReadOnlyAddressBook getAddressBook();
 
     /**
-     * Returns true if a person with the same identity as {@code person} exists in the address book.
+     * Returns true if a person with the same identity as {@code person} exists in MediCLI.
      */
     boolean hasPerson(Person person);
 
     /**
-     * Returns true if a person with the same identity as {@code person} exists in the address book.
+     * Returns true if an appointment with the same identity as {@code appointment} exists in MediCLI.
      */
     boolean hasAppointment(Appointment appointment);
 
@@ -72,19 +72,19 @@ public interface Model {
 
     /**
      * Adds the given person.
-     * {@code person} must not already exist in the address book.
+     * {@code person} must not already exist in MediCLI.
      */
     void addPerson(Person person);
 
     /**
-     * Deletes the given person.
-     * The person must exist in the address book.
+     * Deletes the given appointment.
+     * The appointment must exist in MediCLI.
      */
     void deleteAppointment(Appointment appointment);
 
     /**
-     * Adds the given person.
-     * {@code person} must not already exist in the address book.
+     * Adds the given appointment.
+     * {@code appointment} must not already exist in MediCLI.
      */
     void addAppointment(Appointment appointment);
 
@@ -113,10 +113,15 @@ public interface Model {
      */
     void updateFilteredPersonList(Predicate<Person> predicate);
 
+    /**
+     * Checks if appointment is valid and returns boolean with the answer.
+     *
+     * @param toAdd Appointment to check
+     * @return boolean indicating is appointment is valid.
+     */
     boolean isValidAppointment(Appointment toAdd);
 
     /** Returns an unmodifiable view of the appointment list */
-
     ObservableList<Appointment> getFilteredAppointmentList();
 
     /**