[CS2113T-T12-2] HappyPills #8
Conversation
…iter report writer 1
docs/DeveloperGuide.md
Outdated
| ## 2. Setting Up | ||
|
|
||
| 1. Ensure that you have `Java 11` or later installed in your computer | ||
| 2. Click [here](link to be added later?) to download the latest HappyPills JAR File |
There was a problem hiding this comment.
I suggest linking to the jar file now that you released it! https://github.com/AY1920S2-CS2113T-T12-2/tp/releases/tag/v2.0
docs/DeveloperGuide.md
Outdated
| overwriting any existing strings in the file. | ||
| This is implemented for edit and delete commands as they cannot be appended. | ||
|
|
||
|  |
There was a problem hiding this comment.
Perhaps you could set the grammar checker in the program you're using to make diagrams so that it doesn't underline certain functions in red? It may look more professional overall.
docs/DeveloperGuide.md
Outdated
| overwriting any existing strings in the file. | ||
| This is implemented for edit and delete commands as they cannot be appended. | ||
|
|
||
|  |
There was a problem hiding this comment.
I think you meant to write toSave() instead of tosave()?
docs/DeveloperGuide.md
Outdated
| overwriting any existing strings in the file. | ||
| This is implemented for edit and delete commands as they cannot be appended. | ||
|
|
||
|  |
There was a problem hiding this comment.
I think the f in getformattedPatientString() should be capitalized (i.e. getFormattedPatientString()) to match the name of the function in the source code
docs/DeveloperGuide.md
Outdated
|
|
||
| **Implementation** | ||
|
|
||
|  |
There was a problem hiding this comment.
There is a typo here (Pateint -> Patient)
docs/DeveloperGuide.md
Outdated
|
|
||
| **Implementation** | ||
|
|
||
|  |
There was a problem hiding this comment.
There is a typo below the Parser box (Pateint -> Patient)
docs/DeveloperGuide.md
Outdated
|
|
||
| **Implementation** | ||
|
|
||
|  |
There was a problem hiding this comment.
Like in images/AddPatientSequenceDiagram.jpg, I suggest shortening the activation bars of the methods, as a method cannot remain active after it has returned.
From the CS2113/T website:
docs/DeveloperGuide.md
Outdated
|
|
||
| **Implementation** | ||
|
|
||
|  |
There was a problem hiding this comment.
Like in images/AddPatientSequenceDiagram.jpg, I suggest shortening the activation bars of the methods, as a method cannot remain active after it has returned.
From the CS2113/T website:
docs/DeveloperGuide.md
Outdated
|
|
||
| **Implementation** | ||
|
|
||
|  |
There was a problem hiding this comment.
Like in images/AddPatientSequenceDiagram.jpg, I suggest shortening the activation bars of the methods, as a method cannot remain active after it has returned.
From the CS2113/T website:
docs/DeveloperGuide.md
Outdated
| overwriting any existing strings in the file. | ||
| This is implemented for edit and delete commands as they cannot be appended. | ||
|
|
||
|  |
There was a problem hiding this comment.
Perhaps you could use the same software you used for the previous sequence diagrams to create this diagram as well? This way, all the sequence diagrams in the guide will look similar and it will have a more professional feel.
docs/DeveloperGuide.md
Outdated
| * `Appointment`: This class manages the data of data type Appointment in memory. | ||
| * `MedicalRecords`: This class manages the data of data type MedicalRecord in memory. | ||
|
|
||
| ### 3.2. TextUi Component |
There was a problem hiding this comment.
Do you mean UI component of TextUi component? Might wanna revise as line 82 mentioned UI component instead
docs/DeveloperGuide.md
Outdated
|
|
||
| ### 3.1. Architecture | ||
|
|
||
|  |
There was a problem hiding this comment.
Architecture diagram looks sound, might want to clarify if its called UI component of textUi component
docs/DeveloperGuide.md
Outdated
|
|
||
| ### 3.4. Model Component | ||
|
|
||
|  |
There was a problem hiding this comment.
Should it be a composition rather than an aggregation, since when the patient list is destroyed, the person is destroyed as well? Vice versa for ApptList and PatientRecordList
docs/DeveloperGuide.md
Outdated
|
|
||
| ### 3.4. Model Component | ||
|
|
||
|  |
There was a problem hiding this comment.
Also, should there be an arrow at the end of the line denoting aggregation?
docs/DeveloperGuide.md
Outdated
| **Implementation** | ||
|
|
||
| The activity diagram below summarises the process of executing an `add` command. | ||
|  |
There was a problem hiding this comment.
Does this mean a new parser object and corresponding objects are constructed and later destroyed every time a command is entered?
Also, the activation bar is slightly too long, can create confusion. Slightly shortening it might help in visibility.
docs/DeveloperGuide.md
Outdated
|
|
||
| **Implementation** | ||
|
|
||
|  |
docs/DeveloperGuide.md
Outdated
|
|
||
| **Implementation** | ||
|
|
||
|  |
docs/DeveloperGuide.md
Outdated
| overwriting any existing strings in the file. | ||
| This is implemented for edit and delete commands as they cannot be appended. | ||
|
|
||
|  |
There was a problem hiding this comment.
a missing command for Patient? After toSave()
Should try to remove the red lines as they affect the visibility and overall look of the diagram
Also, should methods be in camelcase? The words are not following the camelcase format
docs/DeveloperGuide.md
Outdated
| it needs to save, by appending it to the back of the text file. | ||
| This provides improved performance as compared to using writeAllToFile(). | ||
|
|
||
|  |
There was a problem hiding this comment.
Unable to display in the website, omit the /docs and it should work
docs/DeveloperGuide.md
Outdated
| `parsePatientFileContent` to convert each line into a patient object and adds it back to the patient map. | ||
| `loadAppointmentFromFile` and `parseAppointmentFileContent` does the same with the appointment file. | ||
|
|
||
|  |
There was a problem hiding this comment.
Unable to display in the website, omit the /docs and it should work
docs/DeveloperGuide.md
Outdated
| ## 2. Setting Up | ||
|
|
||
| 1. Ensure that you have `Java 11` or later installed in your computer | ||
| 2. Click [here](link to be added later?) to download the latest HappyPills JAR File |
There was a problem hiding this comment.
Do ensure that you remember to update the link! The link currently goes nowhere :)
docs/DeveloperGuide.md
Outdated
|
|
||
| ### 3.1. Architecture | ||
|
|
||
|  |
There was a problem hiding this comment.
The rest of your DG does not seem to have information on logging/commons? If it is not referenced in the rest of your DG should it be removed?
docs/DeveloperGuide.md
Outdated
| * `Appointment`: This class manages the data of data type Appointment in memory. | ||
| * `MedicalRecords`: This class manages the data of data type MedicalRecord in memory. | ||
|
|
||
| ### 3.2. TextUi Component |
There was a problem hiding this comment.
Your previous section only mentions Ui component, not TextUi. Is this a new component or are they the same? If they are supposed to be either do ensure to check your DG for consistency!
docs/DeveloperGuide.md
Outdated
|
|
||
| ### 3.3. Logic Component | ||
|
|
||
|  |
There was a problem hiding this comment.
Your Logic and Model diagrams are a bit small on the website, maybe consider either changing the theme or making the text easier to view?
This is what I see on 100% zoom on my computer, I have to zoom in to read the diagram. BUT I do note that the image looks fine on the github repo itself
docs/DeveloperGuide.md
Outdated
|
|
||
| ### 3.3. Logic Component | ||
|
|
||
|  |
There was a problem hiding this comment.
The PatientCommandParser seems to have 2 'creates' attached to the line (denoted in red), if it's for the same line then should one be enough? If it's supposed to be for separate links then do ensure that the labels are attached accordingly.
For the demarcations in green, your arrow lines crisscross each other a lot and are very close to each other which makes it hard/distracting to track, maybe consider spreading them out? Some distinct points are the circled section (very close to each other) and the underlined link (very close to the PatientRecordParser/PatientCommandParser/AppointmentParser boxes)
docs/DeveloperGuide.md
Outdated
| overwriting any existing strings in the file. | ||
| This is implemented for edit and delete commands as they cannot be appended. | ||
|
|
||
|  |
There was a problem hiding this comment.
It would be good to have similar formatting in diagrams! also it would be good to have an image that doesn't have the red squiggly lines under the text. Also I believe that the life lines (sorry I can't remember the exact term) should be the same length of they have not been closed (red circled sections). Alternatively, denote the 'x' sign to indicate that it is finished. Similarly for the following diagrams with the same format
|
|
||
| The following diagram shows how each command interacts with the other classes. | ||
|
|
||
| *writeAllToFile* |
There was a problem hiding this comment.
If this is meant to be a header, try to make it more obvious? It is easy to overlook because it is the same size and design of the surrounding text. Similarly to the following headers with the same design
docs/DeveloperGuide.md
Outdated
| Cons: If the program was to terminate unexpectedly, the deletion may not be reflected in the respective files. | ||
|
|
||
|
|
||
| ### 4.5 Logging |
There was a problem hiding this comment.
Do remember to update your sections 4.5 to 6!
docs/DeveloperGuide.md
Outdated
| |v2.0|Doctor|note down the prescription that I gave my patients|know what side effects are to be expected (based on the patient's current condition)| | ||
| |v2.0|Doctor|record down the symptoms of my patients|check for any persistent health condition| | ||
| |v2.0|Doctor|add Patient's medical records|view the previous reasons for doctor's visits| | ||
| |v2.1|Doctor|?|?| |
There was a problem hiding this comment.
Do remember to update your v2.1 sections!
docs/DeveloperGuide.md
Outdated
| ---------------------|---------------------------------------------------------------------------------------------- | ||
|
|
||
|
|
||
| ## Useful links: |
There was a problem hiding this comment.
A repeated section, you have this in section 7 already.
Issues fix
Add blank spaces for list command textui
Update DG & PPP
Update PPP
Update DG
Update sitinadiah25.md
…into Nyan-HappyPills # Conflicts: # src/main/java/seedu/happypills/model/data/AppointmentMap.java
Update DG? final pls
Fix last min bug
Update PPP
Update DG :(((
…into Nyan-HappyPills
Finalize HAPPYPILLS
@Janicetyy
@itskesin
@NyanWunPaing