Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enhance Medical History capabilities #120

Merged
merged 7 commits into from
Oct 24, 2021
Merged

Conversation

didymental
Copy link

@didymental didymental commented Oct 23, 2021

This PR will add the following capabilities to the MedicalHistory class of the Patient:

  • Automatically recording the date of the medical entry to be the date the medical entry is recorded (i.e. LocalDate.now(Zone.Id.of(Singapore)))
  • Ability on the EditPatientCommandParser to break a patient's medical history into multiple medical entries, based on a CSV delimiter fashion
  • Ability on the ParserUtil to break a patient's medical history into multiple medical entries, based on a CSV delimiter fashion
  • Update test cases to reflect these changes

This PR partially resolves #79

@didymental didymental added this to the v1.3 milestone Oct 23, 2021
@codecov-commenter
Copy link

codecov-commenter commented Oct 23, 2021

Codecov Report

Merging #120 (e697ddb) into master (d901dd5) will increase coverage by 1.65%.
The diff coverage is 80.72%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #120      +/-   ##
============================================
+ Coverage     61.38%   63.04%   +1.65%     
- Complexity      491      516      +25     
============================================
  Files            94       94              
  Lines          1800     1870      +70     
  Branches        190      205      +15     
============================================
+ Hits           1105     1179      +74     
+ Misses          636      619      -17     
- Partials         59       72      +13     
Impacted Files Coverage Δ
...edu/address/logic/commands/EditPatientCommand.java 96.05% <50.00%> (-1.29%) ⬇️
...ava/seedu/address/model/person/MedicalHistory.java 73.07% <72.34%> (-8.75%) ⬇️
...in/java/seedu/address/logic/parser/ParserUtil.java 95.91% <92.30%> (-1.38%) ⬇️
.../java/seedu/address/storage/JsonAdaptedPerson.java 98.27% <93.33%> (-1.73%) ⬇️
...address/logic/parser/EditPatientCommandParser.java 93.33% <100.00%> (+0.74%) ⬆️
src/main/java/seedu/address/model/EntryList.java 75.00% <100.00%> (+75.00%) ⬆️
src/main/java/seedu/address/model/Entry.java 71.42% <0.00%> (+71.42%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d901dd5...e697ddb. Read the comment docs.

Copy link

@joshenx joshenx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would like to check if changing the access modifier of arg multimap affects anything. Attributes should be private.

Also, how is medical history shown on UI now?

@@ -16,7 +16,7 @@
public class ArgumentMultimap {

/** Prefixes mapped to their respective arguments**/
private final Map<Prefix, List<String>> argMultimap = new HashMap<>();
public final Map<Prefix, List<String>> argMultimap = new HashMap<>();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this attribute being changed to public? This violates OOP principles.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshenx thank you! sorry, I changed to public to help the debugging process when I was failing test cases, forgot to change back. Pushed a new commit to fix.

Copy link

@joshenx joshenx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@didymental didymental merged commit 333c526 into master Oct 24, 2021
@didymental
Copy link
Author

@joshenx this is how the MedicalHistory appears on GUI currently. Currently, there's only one medical entry within each medical history of all patients. Once there are more, it'll appear like this, "24 Oct 2021 | lovesick, 25 Oct 2021 | covid positive", separated by commas.

Screenshot 2021-10-24 at 3 16 45 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve functionality of Medical History
3 participants