-
Notifications
You must be signed in to change notification settings - Fork 155
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 step in setup file, fix methods in coffeeShopOrder class #146
Conversation
@@ -86,27 +86,29 @@ public List<String> getFoodItemsForOrder() | |||
public String generateReceipt() |
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.
In the method description, you should specify "Generate a receipt for a customer's food items." The list it's iterating through contains both food and beverage items, yet only food is being covered here - that needs to be specified in the method description, separate from the tests. Also consider renaming the method, "generateFoodItemsReceipt" or similar. Same comment for the two other CoffeeShopOrder tests - this should be consistent in all 3 classes.
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.
make sense
@@ -58,12 +58,20 @@ public void setUp() | |||
} |
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.
Add Americano to the order, to have a variety of items included - all tests should still pass. Same comment for the other two CoffeeShopTest classes, it should be consistent across all 3 classes.
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.
Americano is not a record class so i don't want them to confuse with how to deal with that.
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 shouldn't have an impact on any of the tests here - the food items for order and receipt methods explicitly say to only cover the food case. A list of order items can have any drinks and any foods.
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.
Like you should be able to add Americano to the order in setUp() and all of the tests will pass without being changed.
coffee-shop-kata/SETUP.md
Outdated
that [here](https://www.jetbrains.com/idea/guide/tutorials/working-with-maven/importing-a-project/). | ||
3. To verify that the Java 8 module is set up correctly, | ||
3. To use Java 20 preview features in IntelliJ IDEA, follow the following steps: |
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.
"follow these steps" instead of"follow the following steps" too verbose.
Signed-off-by: Aqsa Malik <[email protected]>
No description provided.