Add option to pay all persons in PayCommand #102
Conversation
| @@ -24,43 +26,98 @@ | |||
| import seedu.address.model.tag.Tag; | |||
|
|
|||
| /** | |||
| * Pays a person identified using it's displayed index from the address book. | |||
| * Pays a person identified using it's displayed index or all persons in the current list from the address book. | |||
There was a problem hiding this comment.
Minor nitpick, but would "Pays a person using their displayed index, or all persons..." be a clearer description?
|
|
||
| /** | ||
| * Constructor for paying all persons in the list. |
There was a problem hiding this comment.
Should this header comment be of the form "Creates..." (Code Quality for comments) instead?
| } | ||
|
|
||
| /** | ||
| * Constructor for paying a specific person in the list. |
There was a problem hiding this comment.
Similar to the above constructor, should the header comment be of the form "Creates..." (Code Quality for comments) instead?
There was a problem hiding this comment.
I have updated the comment, thanks!
|
|
||
| // Pay all persons in the list | ||
| for (Person personToPay: lastShownList) { | ||
| Person paidPerson = createPaidPerson(personToPay); |
There was a problem hiding this comment.
Similar to above, should there be a check to see if the person is actually pending payment?
| throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); | ||
| } | ||
|
|
||
| Person personToPay = lastShownList.get(targetIndex.getZeroBased()); | ||
| // Pay the person of the specified index | ||
| Person personToPay = lastShownList.get(targetIndex.get().getZeroBased()); | ||
| Person paidPerson = createPaidPerson(personToPay); |
There was a problem hiding this comment.
Could this be a potential bug, since there does not seem to be a check if the person is pending payment? (So it is possible that someone who is already paid gets paid again)
There was a problem hiding this comment.
I missed that, thanks for pointing that out! I have added a validity check regarding already paid persons.
Add an option to pay all persons in the current list rather than just one person using PayCommand (resolves #101).