Week 3 Address Book Pull Request #28
Conversation
Static parameters for new function to edit phone number added Test file used for tutorial demonstration removed
Changed "edit phone" command to "edit_phone" for easier usage Added help text into the display help function Modified the expected output file to reflect new changes
Entering the "edit_phone" command with the necessary parameters will now be accepted. Entering invalid indexes like very large numbers or non numbers will no longer be accepted. Persisting Issues: Checking for 2nd parameter (the phone number to be changed to) has not been implemented yet. Actual changing function has not been implemented yet. Test files still work since no test for this scenerio has been added.
Function will now change the phone number of a selected index to a newly specified phone number. Checker will prevent any non numbers from being entered into the phone number field. Test file still works because the new function auto test has yet to be added Outstanding tasks: Update test file to include testing for the Edit Phone Number function
Added test for editting of phone number. Test can be run without problems.
| /** | ||
| * Edits the phone number of a person identified using last displayed index. | ||
| * | ||
| * @param commandArgs full command args string from the user |
There was a problem hiding this comment.
This should be more descriptive, maybe state what the command args should contain?
| final String phoneNumberToChange = splitUserArgs[1].trim(); | ||
| if (!phoneNumberToChange.matches("[0-9]+")) { | ||
| return getMessageForInvalidCommandInput(COMMAND_EDIT_PHONE_WORD, getUsageInfoForEditPhoneCommand()); | ||
| } |
There was a problem hiding this comment.
A lot of checks above here which could have been pulled out into a function that returns boolean, e.g. do if(!isEditPhoneArgsValid(commandArgs) return getMessage..., refer to the other commands. This has the advantage that you can change the behaviour when receiving invalid input to something else more easily.
|
@DarrenDragonLee close PR after reading |
|
Hi @DarrenDragonLee, your pull request title is invalid. For PR sent to satisfy a learning outcome, 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. |
Hi Ewald
Here is the pull request for your review.
@ewaldhew