Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Replenish List #122

Merged
merged 34 commits into from Oct 30, 2019
Merged

Add Replenish List #122

merged 34 commits into from Oct 30, 2019

Conversation

liawsy
Copy link

@liawsy liawsy commented Oct 29, 2019

This resolves #36, #70, #115, #116, #117, #118.

  1. Refactor Item class as XpireItem class and create parent class for Item
  2. Update model to contain ReplenishList and Xpire
  3. Update storage to read and write to ReplenishList and Xpire
  4. Update view to toggle between main list and replenish list (Done by @febee99)
  5. Create new parser to read commands for replenish list (Done by @febee99)
  6. Update logic manager to contain ReplenishParser (Done by @febee99)
  7. Update test cases because of refactoring of classes
  8. Auto-tag expired items
  9. Auto-shift items whose deleted quantity hits 0 into replenish list
  10. Implement functionality to shift items into replenish list and back into main list via ShiftCommand

Todo:

  1. Update Developer Guide
  2. Update User Guide
  3. Implement test cases

@@ -100,6 +100,7 @@
return objectMapper.readValue(json, instanceClass);
}


Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra line here haha.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I didn't leave the extra line here I got an error from Travis 🤔

}

@Override
public CommandResult execute(String commandText) throws CommandException, ParseException {
logger.info("----------------[USER COMMAND][" + commandText + "]");

CommandResult commandResult;
Command command = this.xpireParser.parseCommand(commandText);
if (this.model.getCurrentFilteredItemList() == this.model.getFilteredXpireItemList()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the return values primitives here? Otherwise should call .equals() method.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okays, will update! :)


@Override
public CommandResult execute(Model model) {
requireNonNull(model);
String output = String.format(MESSAGE_SUCCESS, "the");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When will the message be "Displayed all items in the list"? Maybe just say "tracking list" it it's the xpireItem list?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, the message "Displayed all items in the list" is displayed when just "view" is called without a list argument, whereas a specific list will be mentioned if it is specified.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So both lists will be displayed? Okay!

import io.xpire.model.tag.TagComparator;

/**
* Adds a {@code ToBuyItem} to the Replenish List.
Copy link

@febee99 febee99 Oct 29, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi sy, did you perhaps mean Item instead of ToBuyItem here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeps, I have updated it in my latest commit and will push the changes later on 😝

* Adds the given item.
* {@code item} must not already exist in xpire.
* Deletes the given xpireItem.
* The xpireItem must exist in xpire.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps The xpireItem here can be replaced with {@code xpireItem} to match with the rest of the code :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright!

* Adds a item to xpire.
* The item must not already exist in xpire.
* Adds a xpireItem to xpire.
* The xpireItem must not already exist in xpire.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps The xpireItem can be replaced with {@code xpireItem} to match with the rest of the code :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noted! :)

@liawsy liawsy added this to the v1.3 milestone Oct 30, 2019
@liawsy liawsy requested review from febee99 and removed request for febee99 October 30, 2019 07:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

As a user, I can view the list of things to be replenished
4 participants