From 7599e3a541fab55f6e718e68d5bfc6ac070f6cfc Mon Sep 17 00:00:00 2001
From: Lara Blersch <lb210@hdm-stuttgart.de>
Date: Wed, 10 May 2023 17:32:57 +0200
Subject: [PATCH] changed "return false" to "throw exception" and altered
 corresponding tests

---
 .../java/mi/hdm/recipes/CategoryManager.java  | 22 ++++++------
 .../mi/hdm/recipes/IngredientManager.java     | 16 ++++-----
 .../java/mi/hdm/recipes/RecipeManager.java    | 10 ++++--
 .../mi/hdm/shoppingList/ShoppingList.java     |  6 ++--
 .../resources/fxml/shoppingList-view.fxml     | 36 +++++++++++++++++++
 .../java/mi/hdm/recipes/IngredientTest.java   |  4 +--
 .../mi/hdm/recipes/RecipeManagerTest.java     |  2 +-
 7 files changed, 66 insertions(+), 30 deletions(-)
 create mode 100644 src/main/resources/fxml/shoppingList-view.fxml

diff --git a/src/main/java/mi/hdm/recipes/CategoryManager.java b/src/main/java/mi/hdm/recipes/CategoryManager.java
index c217958..73b7492 100644
--- a/src/main/java/mi/hdm/recipes/CategoryManager.java
+++ b/src/main/java/mi/hdm/recipes/CategoryManager.java
@@ -27,31 +27,29 @@ public class CategoryManager {
      * Adds a category if neither the given name nor the color code have been used in a category before.
      * @param name Name for the category that is to be added
      * @param colourCode Color code for the category that is to be added
-     * @return True if the category has been added (meaning no equal category existed before), otherwise false.
+     * @throws InvalidCategoryException if the category already exists
      */
-    public boolean addCategory(String name, int colourCode){
+    public void addCategory(String name, int colourCode){
         Category c = new Category(name, colourCode);
         if (getCategoryByName(name).isPresent()) {
-            log.error("Category: {} not added because it already exists", c.getName());
-            return false;
+            log.error("Category {} not added because it already exists", c.getName());
+            throw new InvalidCategoryException("Category already exists.");
         } else {
             allCategories.add(c);
             log.info("Category {} added successfully.", c.getName());
-            return true;
         }
     }
 
-    public boolean deleteCategory(Category c) {
+    public void deleteCategory(Category c) {
         log.info("Category {} deleted successfully.", c.getName());
-        return allCategories.remove(c);
-
+        if (allCategories.remove(c) == false) {
+            throw new InvalidCategoryException("Category is not listed.");
+        }
     }
 
-    public boolean deleteCategory(String name) {
+    public void deleteCategory(String name) {
         Category c = getCategoryByName(name).orElseThrow(() -> new InvalidCategoryException("Category not found."));
-        log.info("Category {} deleted successfully.", name);
-        return allCategories.remove(c);
-
+        deleteCategory(c);
     }
 
     public List<Category> getAllCategories() {
diff --git a/src/main/java/mi/hdm/recipes/IngredientManager.java b/src/main/java/mi/hdm/recipes/IngredientManager.java
index 09fd6fd..4fa2349 100644
--- a/src/main/java/mi/hdm/recipes/IngredientManager.java
+++ b/src/main/java/mi/hdm/recipes/IngredientManager.java
@@ -7,7 +7,7 @@ import org.apache.logging.log4j.Logger;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
-
+//TODO: check for other "return false" methods which should throw an exceptions instead
 //TODO: make this not a singleton?
 public class IngredientManager {
     private static final Logger log = LogManager.getLogger(IngredientManager.class);
@@ -28,21 +28,19 @@ public class IngredientManager {
      * @param in Ingredient that is to be added
      * @return True if the ingredient has been added (meaning no equal ingredient existed before), otherwise false.
      */
-    public boolean addIngredient(Ingredient in) {
+    public void addIngredient(Ingredient in) {
         if (getIngredientByName(in.getName()).isPresent()) {
             log.error("Ingredient {} not added because it already exists.", in.getName());
-            return false;
-            //TODO: Exception or false?
+            throw new InvalidIngredientException("Ingredient already exists.");
         } else {
             log.info("Ingredient {} added successfully.", in.getName());
             allIngredients.add(in);
-            return true;
         }
     }
 
-    public boolean addIngredient(Measurement unit, String name, NutritionTable nutritionTable) {
+    public void addIngredient(Measurement unit, String name, NutritionTable nutritionTable) {
         Ingredient in = new Ingredient(unit, name, nutritionTable);
-        return addIngredient(in);
+        addIngredient(in);
     }
 
     public Ingredient deleteIngredient(int i) {
@@ -50,14 +48,14 @@ public class IngredientManager {
         return allIngredients.remove(i);
     }
 
-    public boolean deleteIngredient(String name) {
+    public void deleteIngredient(String name) {
         //TODO: check this out, might be a better solution
         /*Optional<Ingredient> ingredient = getIngredientByName(name);
         return ingredient.isPresent() && allIngredients.remove(ingredient);*/
 
         Ingredient ingredient = getIngredientByName(name).orElseThrow(() -> new InvalidIngredientException("No ingredient with name " + name));
         log.info("Ingredient {} deleted successfully.", name);
-        return allIngredients.remove(ingredient);
+        allIngredients.remove(ingredient);
     }
 
     public void clearIngredients() {
diff --git a/src/main/java/mi/hdm/recipes/RecipeManager.java b/src/main/java/mi/hdm/recipes/RecipeManager.java
index 25f05ea..89a9bad 100644
--- a/src/main/java/mi/hdm/recipes/RecipeManager.java
+++ b/src/main/java/mi/hdm/recipes/RecipeManager.java
@@ -47,9 +47,13 @@ public class RecipeManager {
         return recipes.remove(r);
     }
 
-    //TODO: this returns false, other methods throw an exception. whats the best approach?
-    public boolean deleteRecipe(Recipe r) {
-        return recipes.remove(r);
+    // Exception vs return false: What's the best approach?
+    // -> false for normal situations, exceptions for real faults which should not occur
+    public void deleteRecipe(Recipe r) {
+        log.info("Recipe deleted successfully.");
+        if (recipes.remove(r) == false) {
+            throw new InvalidRecipeException("Recipe is not listed.");
+        }
     }
 
     private Optional<Recipe> getRecipeByName(String name) {
diff --git a/src/main/java/mi/hdm/shoppingList/ShoppingList.java b/src/main/java/mi/hdm/shoppingList/ShoppingList.java
index 3fc5c2a..95efe9e 100644
--- a/src/main/java/mi/hdm/shoppingList/ShoppingList.java
+++ b/src/main/java/mi/hdm/shoppingList/ShoppingList.java
@@ -45,9 +45,9 @@ public class ShoppingList {
                 .forEach(element -> {
                     if (element instanceof Ingredient i)
                         addToShoppingList(i);
-                    //TODO: do we want to add recipes recursively?? (see below) -> ask Karin
-                    /*else
-                        addAllToShoppingList((Recipe) element);*/
+                    //add recipes recursively (see below); comment next 2 lines out to avoid adding recipes recursively
+                    else
+                        addAllToShoppingList((Recipe) element);
                 });
     }
 
diff --git a/src/main/resources/fxml/shoppingList-view.fxml b/src/main/resources/fxml/shoppingList-view.fxml
new file mode 100644
index 0000000..4665df8
--- /dev/null
+++ b/src/main/resources/fxml/shoppingList-view.fxml
@@ -0,0 +1,36 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<?import javafx.scene.control.*?>
+<?import javafx.scene.layout.*?>
+<?import javafx.scene.text.*?>
+
+<AnchorPane prefHeight="400.0" prefWidth="600.0" xmlns="http://javafx.com/javafx/17.0.2-ea" xmlns:fx="http://javafx.com/fxml/1" fx:controller="fxml.ShoppingListView">
+   <children>
+      <ToolBar prefHeight="40.0" prefWidth="606.0" style="-fx-background-color: FFFFFF;">
+         <items>
+            <MenuButton contentDisplay="CENTER" mnemonicParsing="false" style="-fx-background-color: FFFFFF; -fx-border-color: D91C1C; -fx-border-radius: 10; -fx-border-width: 2;" text="Menu">
+              <items>
+                <MenuItem mnemonicParsing="false" text="Meal Plan" />
+                <MenuItem mnemonicParsing="false" text="Shopping List" />
+              </items>
+               <font>
+                  <Font name="Inter Regular" size="12.0" />
+               </font>
+            </MenuButton>
+            <TextField alignment="CENTER" editable="false" style="-fx-background-color: FFFFFF;" text="Tasty Pages">
+               <font>
+                  <Font name="Inter Regular" size="12.0" />
+               </font>
+            </TextField>
+         </items></ToolBar>
+      <SplitPane dividerPositions="0.6341059602649006" layoutY="40.0" mouseTransparent="true" prefHeight="360.0" prefWidth="606.0" style="-fx-background-color: FFFFFF;">
+        <items>
+          <AnchorPane minHeight="0.0" minWidth="0.0" prefHeight="360.0" prefWidth="379.0" style="-fx-background-color: FFFFFF; -fx-border-color: D91C1C; -fx-border-radius: 10; -fx-border-width: 4; -fx-border-insets: 15;">
+               <children>
+                  <CheckBox layoutX="78.0" layoutY="42.0" mnemonicParsing="false" prefHeight="17.0" prefWidth="199.0" text="Ingredient1" />
+               </children></AnchorPane>
+          <AnchorPane minHeight="0.0" minWidth="0.0" prefHeight="259.0" prefWidth="223.0" style="-fx-background-color: FFFFFF; -fx-border-color: D91C1C; -fx-border-width: 4; -fx-border-radius: 10; -fx-border-insets: 15;" />
+        </items>
+      </SplitPane>
+   </children>
+</AnchorPane>
diff --git a/src/test/java/mi/hdm/recipes/IngredientTest.java b/src/test/java/mi/hdm/recipes/IngredientTest.java
index 82da1bd..059db89 100644
--- a/src/test/java/mi/hdm/recipes/IngredientTest.java
+++ b/src/test/java/mi/hdm/recipes/IngredientTest.java
@@ -56,10 +56,10 @@ public class IngredientTest {
         underTest.addIngredient(i1);
 
         //when
-        boolean shouldBeTrue = underTest.deleteIngredient(i1.getName());
+        underTest.deleteIngredient(i1.getName());
 
         //expect
-        Assertions.assertTrue(shouldBeTrue);
+        Assertions.assertEquals(0, underTest.getAllIngredients().size());
     }
 
     @Test
diff --git a/src/test/java/mi/hdm/recipes/RecipeManagerTest.java b/src/test/java/mi/hdm/recipes/RecipeManagerTest.java
index 0feea92..1f04731 100644
--- a/src/test/java/mi/hdm/recipes/RecipeManagerTest.java
+++ b/src/test/java/mi/hdm/recipes/RecipeManagerTest.java
@@ -51,7 +51,7 @@ class RecipeManagerTest {
 
     @Test
     public void removingInvalidRecipes() {
-        assertFalse(underTest.deleteRecipe(recipeOne));
+        assertThrows(InvalidRecipeException.class, () -> underTest.deleteRecipe(recipeOne));
         assertThrows(InvalidRecipeException.class, () -> underTest.deleteRecipe("This recipe does not exist"));
     }
 }
-- 
GitLab