[CS2113-T10-2] BookBob #11

kaboomzxc · 2024-09-19T03:09:58Z

BookBob helps General Practitioner Doctors to manage patient details. It is optimised for CLI users, so that they are able to access
information of the patients faster by searching their specific details such as Name or NRIC.

TJW0911 · 2024-10-22T13:55:48Z

src/main/java/bookbob/Main.java

+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+public class Main {


This main method is very long. Perhaps you could consider refactoring the code to separate the functionalities for parsing with a 'parser' package similar to what was done in the IP.

TJW0911 · 2024-10-22T13:58:20Z

src/main/java/bookbob/Main.java

+            case "add":
+                logger.log(Level.INFO, "Processing add command");
+                try{
+                    int nameStart = input.indexOf("n/");
+                    int nricStart = input.indexOf("ic/");
+                    int visitStart = input.indexOf("v/");
+
+                    if (nameStart == -1) {
+                        System.out.println("Please provide the patient's name.");
+                        logger.log(Level.INFO, "Name of the patient is not provided");
+                        break;
+                    }
+
+                    if (nricStart == -1) {
+                        System.out.println("Please provide the patient's NRIC.");
+                        logger.log(Level.INFO, "NRIC of the patient is not provided");
+                        break;
+                    }
+
+                    String visitDateString = input.substring(visitStart + 2).trim();
+                    try {
+                        // Attempt to parse using a standard formatter
+                        DateTimeFormatter formatter = DateTimeFormatter.ofPattern("dd-MM-yyyy HH:mm");
+                        LocalDateTime time = LocalDateTime.parse(visitDateString, formatter);
+                    } catch (DateTimeParseException e) {
+                        System.out.println("Invalid visit date format. Please use 'dd-MM-yyyy HH:mm' format.");
+                        logger.log(Level.INFO, "Invalid visit date format provided");
+                        break;
+                    }
+
+                    commandHandler.add(input, records);
+                    logger.log(Level.INFO, "Successfully processed add command");
+                } catch (Exception e) {
+                    logger.log(Level.WARNING, "Error processing add command", e);
+                    System.out.println("Error in adding patient record, specific error: " + e.getMessage());
+                }
+                break;


Perhaps it could be a good idea to extract the logic in each individual switch cases as a method as some of these switch cases are pretty lengthy and extracting method would make the code more concise and readable. This applies to all switch cases in the main method.

TJW0911 · 2024-10-22T14:07:09Z

src/main/java/bookbob/entity/AppointmentRecord.java

+        List<Appointment> appointments = new ArrayList<>();
+        String[] inputs = input.split("/");
+        String details = inputs[1];
+        if (inputs[0].equals("n")) {


Maybe it would be a good idea to do
String filter = inputs[0];
and then use the filter variable to check the conditions for easier readability.

TJW0911 · 2024-10-22T14:15:55Z

src/main/java/bookbob/functions/FileHandler.java

+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+public class FileHandler {


Maybe you could use slightly different method names to name the different methods in this file. I was confused for a bit when I saw multiple pairs of methods with the exact same method name.

TJW0911 · 2024-10-22T14:17:49Z

src/main/java/bookbob/Main.java

+
+        while (true) {
+            String input = in.nextLine();
+            String[] inputArr = input.split(" ", 2);


Maybe it would be a good idea to name the variable in full as Arr is not that commonly used to refer to Array.

louisjoety

Keep it up! 😺

louisjoety · 2024-10-30T02:15:20Z

docs/NewAppointment.puml

+@startuml
+'https://plantuml.com/sequence-diagram
+


Neat and tidy, very detailed.

You may consider adding colours and hide the footbox 👍

louisjoety · 2024-10-30T02:18:59Z

docs/AfterAppointment.puml

+name = John Doe
+nric = S123A
+date = 18-11-2024
+time = 18:00


Can consider using "" for strings

louisjoety · 2024-10-30T02:25:35Z

docs/NewAppointment.puml

+Doctor -> Main ++
+Main -> Record **
+Main -> AppointmentRecord **
+Main -> FileHandler ++ : initFile(records)
+FileHandler -> Main -- :
+Main -> FileHandler ++ : initFile(appointmentRecords)
+FileHandler -> Main -- :
+Main -> CommandHandler ** :
+Doctor -> Main : appointment("John Doe, S123A, 18-11-2024, 18:00")
+Main -> CommandHandler ++: commandHandler.appointment("John Doe, S123A, 18-11-2024, 18:00")
+CommandHandler -> AppointmentRecord ++ : appointmentRecord.checkAvailability("18-11-2024, 18:00")
+AppointmentRecord -> CommandHandler -- : nextAvailableTime
+group alt [nextAvailableTime == "18:00"]
+    CommandHandler -> Appointment **
+    CommandHandler -> Appointment ++ : new Appointment("John Doe", "S123A", "18-11-2024", "18:00")
+    Appointment -> CommandHandler -- : appointment
+
+    CommandHandler -> AppointmentRecord ++ : appointmentRecords.addAppointment(appointment)
+    AppointmentRecord -> CommandHandler --
+    else
+end
+CommandHandler -> FileHandler ++ : FileHandler.autosave(appointmentRecords)
+FileHandler -> CommandHandler --


You may wish to consider deactivating the activation bar after the completion

louisjoety · 2024-10-30T02:26:15Z

docs/NewAppointment.puml

+@startuml
+'https://plantuml.com/sequence-diagram
+


I found many rounded rectangles instead of rectangles, from my knowledge, we have to use rectangles.

louisjoety · 2024-10-30T02:27:00Z

docs/NewAppointment.puml

+Doctor -> Main : appointment("John Doe, S123A, 18-11-2024, 18:00")
+Main -> CommandHandler ++: commandHandler.appointment("John Doe, S123A, 18-11-2024, 18:00")
+CommandHandler -> AppointmentRecord ++ : appointmentRecord.checkAvailability("18-11-2024, 18:00")
+AppointmentRecord -> CommandHandler -- : nextAvailableTime


I think for any return arrows, it should be --> instead

louisjoety · 2024-10-30T02:28:43Z

docs/AfterAppointment.puml

+Main --> Record
+Main --> AppointmentRecord
+Main --> FileHandler
+Main --> CommandHandler
+Main --> Scanner
+CommandHandler --> FileHandler
+CommandHandler --> AppointmentRecord
+CommandHandler --> Record
+AppointmentRecord --> Appointment


Object diagram well drawn, can consider adding labels to make it clearer

louisjoety · 2024-10-30T02:45:10Z

docs/NewAppointment.puml

+Doctor -> Main ++
+Main -> Record **
+Main -> AppointmentRecord **
+Main -> FileHandler ++ : initFile(records)
+FileHandler -> Main -- :
+Main -> FileHandler ++ : initFile(appointmentRecords)
+FileHandler -> Main -- :
+Main -> CommandHandler ** :
+Doctor -> Main : appointment("John Doe, S123A, 18-11-2024, 18:00")
+Main -> CommandHandler ++: commandHandler.appointment("John Doe, S123A, 18-11-2024, 18:00")


Missing colon in front of the entity name (i.e.":Main" instead of "Main" )

cxc0418 · 2024-10-30T06:38:45Z

docs/DeveloperGuide.md

+| Action | Format | Example |
+|---|---|---|
+| Help | `help` | `help` |
+| Add | `add n/NAME ic/NRIC [p/PHONE_NUMBER] [d/DIAGNOSIS] [m/MEDICATION] [ha/HOME_ADDRESS] [dob/DATE_OF_BIRTH] [v/VISIT_DATE_TIME] [al/ALLERGY] [s/SEX] [mh/MEDICALHISTORY]` | `add n/James Ho ic/S9534567A p/91234567 d/Asthma m/Albuterol ha/NUS-PGPR dob/01011990 v/21-10-2024 15:48 al/Pollen s/Female mh/Diabetes` |


For the appointment command syntax, should the date format dd-mm-yyyy and time format HH:mm be clarified further in the command summary?

cxc0418 · 2024-10-30T06:39:29Z

docs/DeveloperGuide.md

+1. Adding visits
+    1. (Positive) Test Case: `addVisit ic/S9876543A v/21-10-2024 15:48 d/Fever,Cough m/Paracetamol,Cough Syrup` <br>
+       Expected: Visit added to patient's record with diagnoses and medications.


Would providing a few additional examples for variations help reduce input errors?

cxc0418 · 2024-10-30T06:41:26Z

docs/DeveloperGuide.md

+### Data Persistence (Saving and Loading)
+
+1. Automatic Storage
+    1. Test case: Add/edit/delete records, then restart application
+        - Expected: All changes are preserved after restart
+        - Note: Data is automatically saved to `bookbob_data.txt` in the `data` folder (same directory as Bookbob.jar)
+
+2. File Management
+    1. Test case: Manually View saved data
+        - Action: Navigate to `data` folder (same directory as BookBob.jar) and open `bookbob_data.txt`
+        - Note: Do not manually modify the file contents to prevent data corruption. Saving and loading data is automated as long as file is not corrupted.

-## Instructions for manual testing
+    2. Test case: Delete data file and restart application
+        - Action: Delete `bookbob_data.txt` from the `data` folder
+        - Expected: New `bookbob_data.txt` file is automatically generated. The missing text file will not result in any error as files will be generated automatically.
+        - Note: This can be used to start with a fresh database if needed

-{Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing}
+3. Error Handling
+    1. Test case: Corrupt the data file manually, then start application
+        - Expected: Error message is shown about invalid data
+        - Note: BookBob will continue to function
+        - Recovery options:
+            * Option 1: Manually remove corrupted lines from `bookbob_data.txt`
+            * Option 2: Delete `bookbob_data.txt` to start afresh


is it necessary to expand on recovery options when the data file is corrupted? Should we add specific steps or a short troubleshooting guide to assist users in recovering from data corruption errors?

cxc0418 · 2024-10-30T06:45:01Z

docs/DeveloperGuide.md

+The Sequence Diagram for the execution of the appointment feature:
+![img.png](NewAppointmentSD.png)


would it be helpful to label key interactions between objects (e.g., CommandHandler, AppointmentRecord, FileHandler) to clarify their roles?

anlamm · 2024-10-30T06:23:37Z

docs/DeveloperGuide.md

+![img.png](AfterExecutionOD.png)
+
+The Sequence Diagram for the execution of the appointment feature:
+![img.png](NewAppointmentSD.png)


Shouldn't the arrows of creating object point to its activation bar?

anlamm · 2024-10-30T06:36:07Z

docs/DeveloperGuide.md

+
+Object Diagram after execution of appointment feature:
+![img.png](AfterExecutionOD.png)
+


It will be clearer if a class diagram is included.

anlamm · 2024-10-30T06:43:26Z

docs/DeveloperGuide.md

+
+The Sequence Diagram for the execution of the appointment feature:
+![img.png](NewAppointmentSD.png)
+


Shouldn't the return arrow in dash line?

anlamm · 2024-10-30T06:44:35Z

docs/DeveloperGuide.md

+
+Object Diagram after execution of appointment feature:
+![img.png](AfterExecutionOD.png)
+


Isn't the object diagram before execution of application feature and after execution of application feature the same?

anlamm · 2024-10-30T06:47:13Z

docs/DeveloperGuide.md

-{Describe the target user profile}
+Dr Bob is a General Practitioner running his own private clinic.
+He manages everything independently, attending to numerous patients with diverse health concerns each day.  
+The demanding workload and long hours often leave him exhausted and sleep-deprived.  


Maybe a bit too much detail about background?

parasytezz · 2024-10-30T06:22:12Z

docs/AfterExecutionOD.png

Is the before and after execution diagram supposed to be exactly the same?

parasytezz · 2024-10-30T06:28:26Z

docs/NewAppointmentSD.png

should the record object have a constructor?

parasytezz · 2024-10-30T06:31:48Z

docs/NewAppointmentSD.png

same for Appointment record

parasytezz · 2024-10-30T06:35:59Z

docs/NewAppointmentSD.png

Appointment should also have a constructor

parasytezz · 2024-10-30T06:42:36Z

docs/NewAppointmentSD.png

Should the return arrows be dotted

parasytezz · 2024-10-30T06:43:53Z

docs/NewAppointmentSD.png

the activation bar for Main and CommandHandler are not closed

parasytezz · 2024-10-30T06:47:17Z

docs/NewAppointmentSD.png

CommandHandler should call autoSave in FileHandler after getting the return nextAvailableTime

mikolajed

In general, the diagrams follow the confection with no major violations, good job. However, there seems to be too much detail in the DeveloperGuide.md as well as on the diagrams themselves.

mikolajed · 2024-11-02T17:10:37Z

docs/AddVisitSequenceDiagram.puml

+":Main" -> ":Scanner"**: new Scanner()
+activate ":Scanner"
+":Main" <-- ":Scanner": in:Scanner
+deactivate ":Scanner"
+
+":Main" -> ":Records"**: new Records()
+activate ":Records"
+":Main" <-- ":Records": records:Records
+deactivate ":Records"
+
+":Main" -> ":AppointmentRecord"**: new AppointmentRecord()
+activate ":AppointmentRecord"
+":Main" <-- ":AppointmentRecord": appointmentRecord:AppointmentRecord
+deactivate ":AppointmentRecord"
+
+participant ":FileHandler" as FileHandler <<class>>
+":Main" -> FileHandler ++: initFile(records)
+return
+
+":Main" -> FileHandler ++: initFile(appointmentRecord)
+return


There seems to be an overlap with the MainSequenceDiagram.puml. Moving this part to a separate diagram and just referencing it would improve readability, would you agree?

mikolajed · 2024-11-02T17:18:14Z

docs/CommandHandler.puml

+help(): void
+add(input: String, records: Records): void
+list(records: Records): void
+edit(input: String, records: Records): void
+editVisit(input: String, records: Records): void
+delete(nric: String, records: Records): void
+find(input: String, records: Records): void
+exit(input: String): void
+addVisit(input: String, records: Records): void
+Appointment(input: String, appointmentRecord: AppointmentRecord): void
+deleteAppointment(input: String, appointmentRecord: AppointmentRecord): void
+listAppointment(appointmentRecord: AppointmentRecord): void
+findAppointment(input: String, appointmentRecord: AppointmentRecord): void
+deleteAppointment(appointmentRecord: AppointmentRecord): void
+-findNextFieldStart(input: String, currentIndex: int): int


I like that it is clear which component is responsible for what. Wouldn't all of these methods be a bit overwhelming for a reader? Isn't it too detailed?

mikolajed · 2024-11-02T17:23:23Z

docs/Main.puml

+@startuml
+'https://plantuml.com/class-diagram
+
+class Main {
+-{static} logger: Logger {readOnly}
+{static} main(args: String[]): void
+}
+
+class Patient {}
+class Visit {}
+class Main {}
+class Records {}
+class CommandHandler {}
+class AppointmentRecord {}
+
+
+CommandHandler --> Appointment : searches and modifies >
+AppointmentRecord "1" o-- "many" Appointment
+Main --> CommandHandler : calls >
+Main --> FindVisit : calls >
+CommandHandler --> Patient : searches and modifies >
+FindVisit --> Visit : searches >
+Records "1" o-- "many" Patient
+Patient "1" o-- "many" Visit
+@enduml


I really like this diagram is provided, important for understanding other ones.

mikolajed · 2024-11-02T17:45:02Z

docs/DeveloperGuide.md

+> The Object Diagram before the execution of "add" command:
+![img.png](ObjectDiagramBeforeAddPatient.png)
+
+> The Object Diagram after the execution of "add" command:
+![img.png](ObjectDiagramAfterAddPatient.png)


It is not blatantly clear what is being changed, too much detail. Maybe it would be beneficial to put the diagrams side by side?

add author

fix author

…l optional parameters (denoted by square brackets)

…leasingV2.0 Do a final check before releasing v2.0

Fix bugs in the commands

Fix minor typo in Main.java

* 'master' of https://github.com/AY2425S1-CS2113-T10-2/tp: (58 commits) Fix minor typo in Main.java Fixed typo Fix typo issue Fix bugs in the commands Make the error message printed out for find command consistent for all optional parameters (denoted by square brackets) Change the help output to guide the users correctly fix author add author add author update_DG fix CI Main Class Test Create RecordsTest class with JUnit tests to ensure 100% test coverage (class, method, line, branch) Update author tags Delete BookBobTest.java from v1.0 (we shifted all test methods into CommandHandlerTest.java) Create a PatientTest class and add JUnit tests to achieve 100% test coverage (class, method, line and branch) Update AboutUs.md Update AboutUs.md Update princecatt.md Update AboutUs.md ... # Conflicts: # docs/AboutUs.md

Update Main class diagram and fix typo in Jz PPP

"patient record management" header in DG reduce size

…ations about mandatory fields and optional fields

…nly "F", "Female", "M" or "Male" (case-insensitive) are allowed as inputs for sex

…leasingv2.1 Final check before releasing v2.1

Fix minor formatting issues in UG

update JZ PPP

Update PPP

update JZ PPP

Update PPP for Cora Zhang

fix Typos in UG

update JZ PPP

TJW0911 reviewed Oct 22, 2024

View reviewed changes

louisjoety reviewed Oct 30, 2024

View reviewed changes

cxc0418 reviewed Oct 30, 2024

View reviewed changes

anlamm reviewed Oct 30, 2024

View reviewed changes

parasytezz reviewed Oct 30, 2024

View reviewed changes

mikolajed reviewed Nov 2, 2024

View reviewed changes

kaboomzxc and others added 23 commits November 7, 2024 01:08

add author

2ea68dd

Merge pull request #287 from kaboomzxc/master

cc7f9ff

add author

fix author

0f09af9

Merge pull request #288 from kaboomzxc/master

e1539c7

fix author

Change the help output to guide the users correctly

2ab0787

Make the error message printed out for find command consistent for al…

4fe89bc

…l optional parameters (denoted by square brackets)

Merge pull request #290 from yentheng0110/YenTheng-FinalCheckBeforeRe…

bbb00d1

…leasingV2.0 Do a final check before releasing v2.0

Fix bugs in the commands

e92f54c

Fix typo issue

21e30e3

Fixed typo

135a41c

Merge pull request #292 from G13nd0n/master

5defa7b

Fix bugs in the commands

Fix minor typo in Main.java

b8129ad

Merge pull request #293 from yentheng0110/master

d061cfb

Fix minor typo in Main.java

Update the DeveloperGuide based on feedback received

c776614

Extract certain variables used throughout the tests

572d071

Add interface for file operations in FileHandler

25e2a43

Add interface for converting file output

92bdc8c

Modify Patient class to include OutputConversion interface

8c9d3b0

Modify Patient class to include OutputConversion interface

a7202fe

Modify AppointmentRecord class to include FileOperation interface

c9357cb

Modify Record class to include FileOperation interface

d4f007d

Remove unused imports and refactored code

8fca0b2

kaboomzxc and others added 30 commits November 12, 2024 08:00

fix typo JZ PPP

46076eb

update Main Object Diagram

25c27ed

Merge pull request #400 from kaboomzxc/master

c3930b5

Update Main class diagram and fix typo in Jz PPP

"patient record management" header in DG reduce size

7f34f9b

Merge pull request #401 from kaboomzxc/master

f251b22

"patient record management" header in DG reduce size

Modify UG for add command to include input validation section, explan…

2f5c16a

…ations about mandatory fields and optional fields

Modify extractGender(String input) in CommandHandler.java such that o…

90127d3

…nly "F", "Female", "M" or "Male" (case-insensitive) are allowed as inputs for sex

Merge pull request #402 from yentheng0110/YenTheng-FinalCheckBeforeRe…

cfc4c90

…leasingv2.1 Final check before releasing v2.1

Fix minor formatting issues in UG

ce0ff8d

Merge pull request #403 from yentheng0110/master

7bd9b28

Fix minor formatting issues in UG

Update PPP

663ad03

update JZ PPP

66b4b4f

Merge pull request #406 from kaboomzxc/master

c9315d4

update JZ PPP

update JZ PPP

3af81e1

Merge pull request #407 from kaboomzxc/master

3cc16f4

update JZ PPP

Update the enhancement contributed

ee6b995

Merge pull request #405 from G13nd0n/master

804f5e7

Update PPP

update JZ PPP

644b435

Merge pull request #408 from kaboomzxc/master

e02a0fc

update JZ PPP

Update PPP

ac258cf

Update PPP

995812b

Merge pull request #409 from coraleaf0602/master

03c92c3

Update PPP for Cora Zhang

Update coraleaf0602.md

ba7678a

fix Typos in UG

7629020

fix CI

7e9b97f

fix CI

b5a046f

fix CI

b66b9a3

Merge pull request #410 from kaboomzxc/master

3046b63

fix Typos in UG

update JZ PPP

78dfe4e

Merge pull request #411 from kaboomzxc/master

7426ee1

update JZ PPP

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CS2113-T10-2] BookBob #11

[CS2113-T10-2] BookBob #11

kaboomzxc commented Sep 19, 2024

TJW0911 Oct 22, 2024

TJW0911 Oct 22, 2024

TJW0911 Oct 22, 2024

TJW0911 Oct 22, 2024

TJW0911 Oct 22, 2024

louisjoety left a comment

louisjoety Oct 30, 2024

louisjoety Oct 30, 2024

louisjoety Oct 30, 2024

louisjoety Oct 30, 2024

louisjoety Oct 30, 2024

louisjoety Oct 30, 2024

louisjoety Oct 30, 2024

cxc0418 Oct 30, 2024

cxc0418 Oct 30, 2024

cxc0418 Oct 30, 2024

cxc0418 Oct 30, 2024

anlamm Oct 30, 2024

anlamm Oct 30, 2024

anlamm Oct 30, 2024

anlamm Oct 30, 2024

anlamm Oct 30, 2024

parasytezz Oct 30, 2024

parasytezz Oct 30, 2024

parasytezz Oct 30, 2024

parasytezz Oct 30, 2024

parasytezz Oct 30, 2024

parasytezz Oct 30, 2024

parasytezz Oct 30, 2024

mikolajed left a comment

mikolajed Nov 2, 2024

mikolajed Nov 2, 2024

mikolajed Nov 2, 2024

mikolajed Nov 2, 2024

		The Sequence Diagram for the execution of the appointment feature:
		![img.png](NewAppointmentSD.png)


		Object Diagram after execution of appointment feature:
		![img.png](AfterExecutionOD.png)

		@startuml
		'https://plantuml.com/sequence-diagram

		@startuml
		'https://plantuml.com/sequence-diagram

[CS2113-T10-2] BookBob #11

Are you sure you want to change the base?

[CS2113-T10-2] BookBob #11

Conversation

kaboomzxc commented Sep 19, 2024

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

louisjoety 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

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

mikolajed 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