Update DeleteCommand to delete by index or name #29
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #29 +/- ##
============================================
- Coverage 75.26% 73.89% -1.37%
- Complexity 419 445 +26
============================================
Files 71 74 +3
Lines 1338 1444 +106
Branches 126 147 +21
============================================
+ Hits 1007 1067 +60
- Misses 301 339 +38
- Partials 30 38 +8 ☔ View full report in Codecov by Sentry. |
| model.deletePerson(personToDelete); | ||
| return new CommandResult(String.format(MESSAGE_DELETE_PERSON_SUCCESS, Messages.format(personToDelete))); | ||
| } else { | ||
| throw new CommandException("Person with the name \"" + targetName + "\" not found."); |
There was a problem hiding this comment.
For greater consistency, would it be better to abstract the String and declare it at the top of the file as a static final variable, similar to MESSAGE_DELETE_PERSON_SUCCESS?
| throw new CommandException("Person with the name \"" + targetName + "\" not found."); | ||
| } | ||
| } else { | ||
| throw new CommandException("Please provide the correct index or the full name of the person!"); |
There was a problem hiding this comment.
Same as the comment above, but could be declared in Messages.java like MESSAGE_INVALID_PERSON_DISPLAYED_INDEX which was used in line 51
| String nameString = name.toString(); | ||
| return new DeleteCommand(null, nameString); | ||
| } else { | ||
| throw new ParseException("Unexpected error occurred while parsing DeleteCommand."); |
There was a problem hiding this comment.
Same as the comments above!
| @@ -29,8 +30,9 @@ public class DeleteCommandTest { | |||
|
|
|||
| @Test | |||
| public void execute_validIndexUnfilteredList_success() { | |||
| String dummyName = " "; | |||
There was a problem hiding this comment.
Since dummyName is used multiple times throughout this file, it could be declared as a static variable for reuse?
cleowenxuan
merged commit 801feda
into
AY2324S2-CS2103T-T11-3:master
Fix #3
Modify the ParserUtil to include a new method called parseNameIndex. This method takes in the string argument and returns either a String name or String index to delete the contacts based on index or name.