Create view command #119

AaronJT1 · 2023-10-23T03:57:29Z

Summary of change

Create all components of the View patient by Ic number functionality and its testing

Key changes

`ViewCommand.java`

Created ViewCommand.java which contains logic for viewing a patient
Create its testing as well to ensure functionality

`ViewCommandParser.java`

Created ViewCommandParser.java which contains logic for parsing a user input relating to View command
Create its testing as well to ensure functionality

`PatientWithIcNumberPredicate.java`

Created PatientWithIcNumberPredicate.java which is a predicate containing logic to filter patients by IcNumber
Create its testing as well to ensure functionality

`PatientWithFieldNotFoundException.java`

Created PatientWithFieldNotFoundException.java which is an exception thrown when user inputs a field that does not exist in any patients
Create its testing as well to ensure functionality

View Command Integration

Integrated the View Command functionality into the program by updating AddressBookParser.java

Currently, there is no predicate to test for patients with a certain Ic number It is important to create a predicate for that scenario so that the filtered list could find the patient with the ic number Add PatientWithNumberPredicate.java and PatientWithNumberPredicateTest.java

Currently, there is no exception which handles a patient with a certain field not being found It is important to catch this improper user input as an exception so as to handle it well, in this case, to output a message to warn user. Create PatientWithFieldNotFoundException.java together with ViewCommand.java and ViewCommandTest.java

Currently, there is ViewCommand.java, however no parser object to create that Command object It is important to maintain the abstraction between command and parser Create ViewCommandParser.java to parse user input and create ViewCommand object

Currently, there is ViewCommandParser.java but no test cases for it Test case is needed to ensure the workability of the parser. Create ViewCommandParserTest.java and fix the relevant checkstyles issues

Currently, the github CI/CD is failing due to checkstyle issues not found by gradle checkstyle test It is important to ensure the checkstyle is met Fix checkstyle issues around the code base

Currently, the developer guide does not contain the implementation philosophy of the view patient feature It is important to constantly update the developer guide of new features to prevent excessive workload towards end of milestone Update DeveloperGuide.md to include View patient feature

Currently, there are no logging mechanism to track code events and no assertions to ensure assumptions are true. Logging is important to allow for ease of debugging and tracking of users actions, assertion on other hand help reduce developer bugs Add logging and assertions to the View and Add patient features

Currently, the test case on toString method is failing It is important to ensure that unit testing like on the toString method is successful Fix PatientTest.java

Currently, the add and view features in the userguide are outdated. It is important to continously update the user guide with new changes so that users can be informed of changes. Update UserGuide.md to improve documentation on add and view feature

wujy28

LGTM! Just a few minor comments here and there that can be addressed in the future. Good job on maintaining code quality!

wujy28 · 2023-10-25T16:32:27Z

docs/DeveloperGuide.md

+* **Alternative 1 (current choice):** Utilize current filteredPatientList display to
+  display the patient.
+    * Pros: Easy to implement.
+    * Cons: Similar way in displaying of patient(s) may lead to confusion between commands.


Mm can I clarify what do you mean by this?

Currently how filteredPatientList works is by taking in a predicate, going through all patients and showing those who meet the predicate. So would be utilizing that feature display my view patient feature

wujy28 · 2023-10-25T16:33:50Z

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

@@ -18,6 +18,8 @@ public class Messages {
    public static final String MESSAGE_PATIENTS_LISTED_OVERVIEW = "%1$d patients listed!";
    public static final String MESSAGE_DUPLICATE_FIELDS =
                "Multiple values specified for the following single-valued field(s): ";
+    public static final String MESSAGE_PATIENT_LISTED_SUCCESS = "Patient found!!";


This is just a small nitpick but perhaps it'll be more formal/professional to just use 1 exclamation point 😅

Will change it!

wujy28 · 2023-10-25T16:48:20Z

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

-
+    public static final Prefix[] RELEVANT_PREFIXES_WITHOUT_TAG = new Prefix[]{PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL
+        , PREFIX_GENDER, PREFIX_IC_NUMBER, PREFIX_BIRTHDAY, PREFIX_ADDRESS};
+    public static final Prefix[] REQUIRED_PREFIXES = new Prefix[]{PREFIX_NAME, PREFIX_IC_NUMBER};


Thanks for remembering to add ic number as a required prefix!

No worries!!

wujy28 · 2023-10-25T17:02:33Z

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

+     * @throws PatientWithFieldNotFoundException If user enters a field not present in any existing patients
+     */
+    @Override
+    public CommandResult execute(Model model) throws PatientWithFieldNotFoundException {


Hmm just to clarify, your implementation of the view command works like find by name, right? In that it filters the list for the patient with the matching ic number? In the event that a patient has yet to be selected in the ui's displayed list (this happens when the app is first opened and no patient is selected yet, after selecting I believe one patient on the list will always be selected), is the matching patient's record still automatically displayed on the right side of the ui? Or is it still blank (no patient selected).
If the case is that the patient's record is not automatically displayed after entering the command, it is still totally fine but the user will still have to click and select the card from the ui list, contradicting our 'no clicks/gui interactions' 'rule' (for the first time only). This is honestly quite a minor thing HAHA

Hi yes the patient will be displayed on the right side even on startup with no patient displayed. And the user would have to click on it to view details as well. Perhaps if we want to make it more seamless, ie show straightaway, i could get the UI in charge to integrate!

wujy28 · 2023-10-25T17:10:01Z

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

+            assertEquals(e.getMessage(),
+                MESSAGE_UNABLE_TO_FIND_PATIENT_WITH_FIELD + "Ic Number : " + testIcNumber1.value);
+        }
+        assertTrue(exceptionThrown);


If I'm not mistaken, you can also use assertThrows!

wujy28 · 2023-10-25T17:11:53Z

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

+        }
+        assertTrue(exceptionThrown);
+    }
+


Might also be good to consider cases where each argument is null?

RiyaMehta2211

Good job overall with the changes implemented!

RiyaMehta2211 · 2023-10-26T13:36:53Z

docs/UserGuide.md

Good job updating the UG with our new features!

RiyaMehta2211 · 2023-10-26T13:40:11Z

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

I see that you throw PatientWithFieldNotFound Exception, which is good. This helps us account for scenarios where a null can be returned, and throwing exceptions for such invalid inputs helps us prevent the system from breaking!

RiyaMehta2211 · 2023-10-26T13:44:41Z

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

        Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get());
        Phone phone = new Phone(Phone.getDefaultPhone());
        Email email = new Email(Email.getDefaultEmail());
        Gender gender = new Gender(Gender.getDefaultGender());
-        IcNumber icNumber = new IcNumber(IcNumber.getDefaultIcNumber());
+        IcNumber icNumber = new IcNumber(argMultimap.getValue(PREFIX_IC_NUMBER).get());


Yup agree with the change here since IC Number is a required prefix just like name, and cannot be subject to a random default value

RiyaMehta2211 · 2023-10-26T13:48:31Z

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

@@ -154,6 +155,7 @@ public static Birthday parseBirthday(String birthday) throws ParseException {
     */
    public static IcNumber parseIcNumber(String icNumber) throws ParseException {
        requireNonNull(icNumber);
+        icNumber = icNumber.toUpperCase();


Good job with these minor details eg (making some attributes like Gender and IC Number) case insensitive. This definitely accounts for unexpected scenarios of inputs while ensuring the system works as expected!

AaronJT1 added 4 commits October 22, 2023 12:18

Create ViewCommandParserTest.java and fix checkstyles issues

41e132e

Currently, there is ViewCommandParser.java but no test cases for it Test case is needed to ensure the workability of the parser. Create ViewCommandParserTest.java and fix the relevant checkstyles issues

AaronJT1 added the priority.High Must do label Oct 23, 2023

AaronJT1 added this to the v1.3 milestone Oct 23, 2023

AaronJT1 requested review from RiyaMehta2211 and wujy28 October 23, 2023 03:57

AaronJT1 self-assigned this Oct 23, 2023

AaronJT1 added 5 commits October 23, 2023 14:08

Fix minor checkstyle issues

c81fdc0

Currently, the github CI/CD is failing due to checkstyle issues not found by gradle checkstyle test It is important to ensure the checkstyle is met Fix checkstyle issues around the code base

Fix PatientTest.java error

ebf1a54

Currently, the test case on toString method is failing It is important to ensure that unit testing like on the toString method is successful Fix PatientTest.java

wujy28 approved these changes Oct 25, 2023

View reviewed changes

AaronJT1 merged commit 5cc46d7 into AY2324S1-CS2103T-T14-2:master Oct 26, 2023
0 of 3 checks passed

RiyaMehta2211 reviewed Oct 26, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create view command #119

Create view command #119

AaronJT1 commented Oct 23, 2023

wujy28 left a comment

wujy28 Oct 25, 2023

AaronJT1 Oct 26, 2023

wujy28 Oct 25, 2023

AaronJT1 Oct 26, 2023

wujy28 Oct 25, 2023

AaronJT1 Oct 26, 2023

wujy28 Oct 25, 2023

AaronJT1 Oct 26, 2023

wujy28 Oct 25, 2023

AaronJT1 Oct 26, 2023

wujy28 Oct 25, 2023

RiyaMehta2211 left a comment

RiyaMehta2211 Oct 26, 2023

RiyaMehta2211 Oct 26, 2023

RiyaMehta2211 Oct 26, 2023

RiyaMehta2211 Oct 26, 2023

Create view command #119

Create view command #119

Conversation

AaronJT1 commented Oct 23, 2023

Summary of change

Create all components of the View patient by Ic number functionality and its testing

Key changes

ViewCommand.java

ViewCommandParser.java

PatientWithIcNumberPredicate.java

PatientWithFieldNotFoundException.java

View Command Integration

wujy28 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RiyaMehta2211 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

`ViewCommand.java`

`ViewCommandParser.java`

`PatientWithIcNumberPredicate.java`

`PatientWithFieldNotFoundException.java`