[W5][M11-1]Wang Jiannan - #25
Conversation
![[warheade]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…add when it does not exist It might be useful in real life scenarios
huyidi
left a comment
huyidi
left a comment
There was a problem hiding this comment.
Looking good. Edit would be a useful command.
|
A very good feature. I think if the example could show the before and after will be better in illustrating the usage of the function |
okkhoy
left a comment
okkhoy
left a comment
There was a problem hiding this comment.
- Do take note of the coding standards and adhere to it when you write code.
- Always remember to update the documentation and tests when you add any new features.
- Without documentation, your users will not be aware of the new feature
- Without tests, you are at a risk of regressions
|
|
||
| import seedu.addressbook.common.Messages; | ||
| import seedu.addressbook.data.exception.IllegalValueException; | ||
| import seedu.addressbook.data.person.*; |
There was a problem hiding this comment.
Do not import everything using *, import only necessary classes
| */ | ||
| public class EditCommand extends Command{ | ||
| public static final String COMMAND_WORD = "edit"; | ||
| public static final String MESSAGE_USAGE = COMMAND_WORD + ": Edits a contact in the address book. " |
| + " John Doe p/98765432 e/johnd@gmail.com a/311, Clementi Ave 2, #02-25 t/friends t/owesMoney"; | ||
|
|
||
| public static final String MESSAGE_SUCCESS = "Contact is edited: %1$s"; | ||
| //public static final String MESSAGE_NONEXISTENT_PERSON = "This person does not exist in the address book"; |
There was a problem hiding this comment.
Eliminate unused code and do not retain them commented, you can always get it back from previous versions if necessary.
| try { | ||
|
|
||
| final ReadOnlyPerson target = getTargetPerson(); | ||
| System.out.println("fk"); |
There was a problem hiding this comment.
Inappropriate code! Use the debugger instead to trace the program flow!
| } catch (PersonNotFoundException pnfe) { | ||
| return new CommandResult(Messages.MESSAGE_PERSON_NOT_IN_ADDRESSBOOK); | ||
| } | ||
| catch (DuplicatePersonException dpethisisuseless){ |
There was a problem hiding this comment.
Coding standard violation here. (2 counts!, can you identify them?)
|
@warheade |
5 participants
No description provided.