[W5][W17-3]Jeremy Loye #160
Conversation
|
For consistency, it should be "We hope to see you again soon!" The h in "hope" should be lower case. |
|
Great that you updated expected.txt after making the changes |
|
Removing the double negatives makes the code easier to read and understand. |
| @@ -29,10 +29,10 @@ public ViewAllCommand(int targetVisibleIndex) { | |||
| public CommandResult execute() { | |||
| try { | |||
| final ReadOnlyPerson target = getTargetPerson(); | |||
| if (!addressBook.containsPerson(target)) { | |||
There was a problem hiding this comment.
This here is not an example of double negative. Double negative refers to having multiple negations in the conditional (e.g. !isNotFound), and does not refer to the case of using a negated condition to represent an unusual branch.
Instead, this is a guard clause, which is used here to detect an unusual condition (i.e. the target not being found in the address book). The reason for doing this is to make the 'happy path' prominent (i.e. the target being found). You can refer to the textbook chapter: https://nus-cs2103-ay1819s2.github.io/cs2103-website/se-book-adapted/chapters/codeQuality.html.
| @@ -237,15 +237,15 @@ private int parseArgsAsDisplayedIndex(String args) throws ParseException, Number | |||
| */ | |||
| private Command prepareFind(String args) { | |||
| final Matcher matcher = KEYWORDS_ARGS_FORMAT.matcher(args.trim()); | |||
| if (!matcher.matches()) { | |||
There was a problem hiding this comment.
No need to refactor this. Refer to the other comment about guard clauses.
-Refactored Parser prepareFind method to remove double negative
-Refactored viewAllCommand execute method to remove double negative
-Changed MESSAGE_GOODBYE message from "Good bye!" to "Good bye! We Hope to see you again soon!"