Add Delete Command #93
Conversation
| try { | ||
| this.recipeList.remove(recipe); | ||
| } catch (RecipeNotFoundException e) { | ||
| throw new RecipeNotFoundException(); |
There was a problem hiding this comment.
It should be fine not catching the exception here considering the exception was thrown in the previous level. It's a runtime exception so we don't necessarily need to catch it, same as other runtime exceptions
There was a problem hiding this comment.
I think throwing an exception is fine so that the code can catch and display the corresponding error if needed
| // public void removeRecipe_recipeNotInRecipeBook_throwsRecipeNotFoundException() { | ||
| // recipeBook.addRecipe(COOKIES); | ||
| // assertThrows(RecipeNotFoundException.class, () -> recipeBook.removeRecipe(SPONGECAKE.getId())); | ||
| // } |
There was a problem hiding this comment.
These tests could be ammended by removing the getId() method call
| // Messages.format(ingredientToDelete)); | ||
| // | ||
| // ModelManager expectedModel = new ModelManager(model.getInventory(), new UserPrefs()); | ||
| // expectedModel.deleteIngredient(ingredientToDelete); |
There was a problem hiding this comment.
Not sure if this actually checks that the item is missing because how the expected model is generated uses the same function of deleteIngredient. deleteIngredient function from the model could be faulty.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #93 +/- ##
============================================
- Coverage 66.41% 64.93% -1.49%
+ Complexity 459 456 -3
============================================
Files 83 85 +2
Lines 1587 1614 +27
Branches 133 134 +1
============================================
- Hits 1054 1048 -6
- Misses 483 516 +33
Partials 50 50
☔ View full report in Codecov by Sentry. |
No description provided.