[W5][T12-4]Lau Wei Tang #169
Conversation
…classes. Updated the input.txt and expected.txt files to match the changes to the code. Also updated the JUnit tests to ensure that the tests reflect the correctness of the changes. Added new rule in AddCommand.java to include that the Person Address must be of this format: block, unit, street, postalcode. Updated the UserGuide to reflect the changes.
|
Hi @WeiTangLau, your pull request title is invalid. For PR sent as submission of learning a topic/graded exercise, the PR name should be in the format of For team PR, the PR name should be in the format of Please follow the above format strictly and edit your title for reprocessing. Note: this comment is posted by a bot. If you believe this is done in error, please create an issue at nus-se-pr-bot and add a link to this PR. |
|
Nice way of changing it up, seems simple but you had to make changes for the storage and most of the testing. Definitely an enhancement to the addressbook |
|
Nice clean & neat PR! I also like the way you listed the modifications in the PR message. However, I think some of the commit messages can be delivered in a more concise manner: Links for more about writing a good commit message: Overall, greatest of job! #LATB |
| public final String value; | ||
|
|
||
| private boolean isPrivate; |
There was a problem hiding this comment.
Use blank lines sparingly. It's good to have blank lines to separate logical code blocks, but too many blank lines will increase the height of the code, which increases the amount of scrolling.
| private String postalCodeNumber; | ||
|
|
||
| public PostalCode(String postalCodeNumber) { | ||
| this.postalCodeNumber = postalCodeNumber; |
There was a problem hiding this comment.
A possible improvement is to utilize regex so that the postal code must be of the form "Sxxxxxx", where x must be a digit from 0 to 9.
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object other) { | ||
| return other == this // short circuit if same object | ||
| || (other instanceof Address // instanceof handles nulls | ||
| && this.value.equals(((Address) other).value)); // state check | ||
| && this.value.equals(((Address) other).value)); |
There was a problem hiding this comment.
Good catch! However, the indentation for wrapped lines should be 8 spaces.
| * | ||
| * @param trimmedAddress the String to be operated on | ||
| */ | ||
| public Block assignBlock(String trimmedAddress) { |
There was a problem hiding this comment.
Consider using a different name for this method, as the block is not in fact assigned, but rather extracted out from the trimmed address.
Modified Address class to include attributes of Block, Unit, Street and PostalCode classes
Updated Tests cases for JUnit tests and IO tests
Updated User Guide to reflect the changes
Modified the AddCommand.java file to enforce the new input method of Person Address