-
Notifications
You must be signed in to change notification settings - Fork 131
Magnus Krumbacher #134
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
base: main
Are you sure you want to change the base?
Magnus Krumbacher #134
Conversation
…ly in the domain-model.md
…ng errors in domain-model.md Last commit for core exercises
…to be able to test properly
| public Item createItem(String id) { | ||
| this.id = id; | ||
| if (this.inventory.containsKey(this.id)) { | ||
| if (this.id.startsWith("B")) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You may want to make this case-insensitive
| //Max capacity is 5, so adding should fail after 5 elements are present in the basket list | ||
| Basket basket = new Basket(); | ||
| ItemFactory factory = new ItemFactory(); | ||
| Item item = factory.createItem("BGLP"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be better to have an overload for the factory method that creates X number of times: create(ItemType, number)
| //This methods will allow multiple discounts to be applied and likely got this ugly because of it | ||
| //I.e. if there are 18 bagels of the same type, first the 12 bagel discount is given and then the 6 bagel discount for the remaining bagles | ||
| //The coffee discount needs "stand-alone" bagels, so not bagles that are already part of another discount calculation | ||
| public void checkDiscounts() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can become hard to maintain if other types of discounts are added in hje future. I think it belongs in its own class, and could do with further decomposition into smaller methods.
|
|
||
| } | ||
|
|
||
| public void checkItemTypes() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This method is called 'checkItemTypes' but it also adds items which makes the name a bit misleading
No description provided.