Add date of birth field #42
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 7 commits
October 1, 2020 02:35
* 'master' of https://github.com/AY2021S1-CS2103T-F13-3/tp: Update User Guide Add foojingyi profile photo
* 'master' of https://github.com/AY2021S1-CS2103T-F13-3/tp: Update homepage for project website Update DG Fix bug in NameContainsKeywordsPredicateTest.java after adding Weight field Fix bug in Person class constructor Update more existing tests to include weight field Update existing tests to support new Weight field, Add 3 new tests for Weight class under WeightTest.java add weight field to person
* master: Update homepage for project website Update DG Fix bug in NameContainsKeywordsPredicateTest.java after adding Weight field Fix bug in Person class constructor Update more existing tests to include weight field Update existing tests to support new Weight field, Add 3 new tests for Weight class under WeightTest.java add weight field to person
wang-jun-hao
reviewed
Oct 1, 2020
Comment on lines
+37
to
+38
| public static final String VALID_DOB_AMY = "14-02-1997"; | ||
| public static final String VALID_DOB_BOB = "11-11-2001"; |
There was a problem hiding this comment.
Perhaps it is better to name it VALID_DATE_OF_BIRTH_AMY and VALID_DATE_OF_BIRTH_BOB to keep it consistent with the current style, where the full class name is used instead of abbreviations?
Comment on lines
+56
to
+57
| public static final String DOB_DESC_AMY = " " + PREFIX_DOB + VALID_DOB_AMY; | ||
| public static final String DOB_DESC_BOB = " " + PREFIX_DOB + VALID_DOB_BOB; |
There was a problem hiding this comment.
Perhaps it is better to name it DATE_OF_BIRTH_DESC_AMY and DATE_OF_BIRTH_DESC_BOB?
| @@ -65,6 +70,7 @@ | |||
|
|
|||
| public static final String INVALID_IC_DESC = " " + PREFIX_IC + "A222223HH"; | |||
| public static final String INVALID_NAME_DESC = " " + PREFIX_NAME + "James&"; // '&' not allowed in names | |||
| public static final String INVALID_DOB_DESC = " " + PREFIX_DOB + "31/12/95"; // '-' should be used instead of '/' | |||
There was a problem hiding this comment.
Maybe it is better to use the full class name for clarity?
A good case of invalid check since '/' is quite a common way of inputting dates.
src/test/java/seedu/address/logic/parser/AddCommandParserTest.java
Outdated
Show resolved
Hide resolved
src/test/java/seedu/address/logic/parser/EditCommandParserTest.java
Outdated
Show resolved
Hide resolved
Remove trailing whitespaces for checkstyle
Remove trailing whitespace on line 51
Wrap to keep within 120 chars
Remove trailing whitespace on line 48
Checkstyle
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added Date Of Birth class and support for a date of birth field.
Resolves #7 .