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 additional attributes to Item class and parameters to add methods #53

Merged
merged 14 commits into from
Mar 17, 2024

Conversation

imanamirshah
Copy link

No description provided.

@imanamirshah imanamirshah added this to the v1.0 milestone Mar 17, 2024
Copy link

@nkotaa nkotaa left a comment

Choose a reason for hiding this comment

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

Apart from a few conciseness issues, looks good. Let's work on it together!

Comment on lines +6 to +9
private final int itemQuantity;
private final String itemExpirationDate;
private final double itemSalePrice;
private final double itemCostPrice;
Copy link

Choose a reason for hiding this comment

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

There might be too many variables. We could look into defining sub-classes of Item to keep this list minimal in the future. Ex. Perishable extends Item

Comment on lines +18 to +27
public AddCommand(ItemList itemList, String itemName, String itemDescription, int itemQuantity,
String itemExpirationDate, double itemSalePrice, double itemCostPrice) {
super(itemList);
this.itemName = itemName;
this.itemDescription = itemDescription;
this.itemQuantity = itemQuantity;
this.itemExpirationDate = itemExpirationDate;
this.itemSalePrice = itemSalePrice;
this.itemCostPrice = itemCostPrice;

Copy link

Choose a reason for hiding this comment

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

This constructor appears needlessly verbose. Perhaps we could define it with an Item parameter instead. i.e public AddCommand(Item newItem)

@nkotaa nkotaa merged commit 1b6bf8c into AY2324S2-CS2113T-T09-2:master Mar 17, 2024
3 checks passed
@YHWong20 YHWong20 linked an issue Mar 19, 2024 that may be closed by this pull request
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 add new items
2 participants