[CS2113-F11-3] PantryPal#29
Conversation
sevenseasofbri
left a comment
sevenseasofbri
left a comment
There was a problem hiding this comment.
Very good job team! I think the code is well structured and follows good code quality practices for the most part. I have briefly gone over and added some comments. Let me know if you have any questions.
|
|
||
| public class Parser { | ||
|
|
||
| public Command parse(String input) { |
There was a problem hiding this comment.
function is >30LOC so try to break it up into multiple functions/refactor similar portions of code into new methods.
| return new RemovePlan(index); | ||
| case "addRecipe": | ||
| return new AddRecipe(); | ||
| case "viewRecipe": //Add error handling for empty recipe field |
There was a problem hiding this comment.
Avoid writing comments like they are notes to the programmer themselves.
| import java.util.Map; | ||
| import java.util.Scanner; | ||
|
|
||
| public class IngredientInventory { |
There was a problem hiding this comment.
For all functions in this class, the method header comments need to be of the following form:
From java coding standard: In method header comments, the first sentence should start in the form Returns ..., Sends ..., Adds ... etc. (not Return or Returning etc.).
It is also better if you write them JavaDoc style.
| } | ||
| } | ||
|
|
||
| public static void main(String[] args) { |
There was a problem hiding this comment.
also a long function, consider breaking it into parts/refactoring relevant code.
| /* | ||
| * | ||
| * @param planIndex refers to the plan slot in a preset | ||
| */ | ||
|
|
||
| /* | ||
| public void AddPlanToCalendar(int planIndex){ | ||
| plans.add(planPresets.get(planIndex)); | ||
| } | ||
| */ | ||
|
|
||
| /* | ||
| * | ||
| * @param planIndex refers to the plan in this calendar | ||
| */ | ||
|
|
||
| /* | ||
| public void RemovePlanFromCalendar(int planIndex){ | ||
| plans.remove(planIndex); | ||
| } | ||
| */ |
There was a problem hiding this comment.
Avoid commenting out dead code. Remove if it is not needed.
| public class Calendar { | ||
| String userName; | ||
| int selectedPlan; | ||
| //ArrayList<MealPlan> plans; |
There was a problem hiding this comment.
Avoid commenting out dead code. Remove if it is not needed.
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| //Return True if 2 object has the same step number |
There was a problem hiding this comment.
| //Return True if 2 object has the same step number | |
| // Return True if 2 object has the same step number |
|
|
||
| public void editRecipe(String command) { | ||
|
|
||
| // Splitting into parts |
There was a problem hiding this comment.
Redundant comment, i think it is already clear from the code.
|
|
||
| for (Map.Entry<String, Double> entry : alerts.entrySet()) { | ||
| String ingredientName = entry.getKey(); | ||
| double threshold = entry.getValue(); | ||
|
|
||
| // Retrieve the ingredient details from the inventory | ||
| Ingredient ingredient = ingredients.get(ingredientName); | ||
| if (ingredient != null && ingredient.getQuantity() < threshold) { | ||
| double required = threshold - ingredient.getQuantity(); | ||
|
|
||
| // Update existing item if present; if not, add a new item. | ||
| boolean updated = shoppingList.updateItem(ingredientName, required, ingredient.getUnit()); | ||
| if (!updated) { | ||
| shoppingList.addItem(new ShoppingListItem(ingredientName, required, ingredient.getUnit())); | ||
| } |
There was a problem hiding this comment.
3-levels of indentation, consider refactoring internal levels .
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class ShoppingList { |
There was a problem hiding this comment.
Similarly for this class,
In method header comments, the first sentence should start in the form Returns ..., Sends ..., Adds ... etc. (not Return or Returning etc.)
|
|
||
| 4. **Error Handling**: The Parser feature includes error handling to manage invalid input and provide feedback to the user. If the input cannot be parsed or if the command is not recognized, an appropriate error message is displayed. | ||
|
|
||
| # Sequence Diagram |
There was a problem hiding this comment.
Entity should have a : beside its name
| - **Unit Management**: Ensures valid unit selection and provides functionality for unit conversion. | ||
|
|
||
|
|
||
| ## Ingredient Inventory Feature |
There was a problem hiding this comment.
Each class section should follow the same format/structure
| - **Deleting Ingredients**: Users can remove ingredients from the inventory, ensuring accurate stock representation. | ||
| - **Low-Stock Alerts**: The system supports configurable alerts to notify users when ingredients are running low. | ||
|
|
||
| ### Sequence Diagram |
There was a problem hiding this comment.
Font size should be consistent too. Nevertheless, good details overall! Covers dependencies and core functionalities
ashleyang2001
left a comment
ashleyang2001
left a comment
There was a problem hiding this comment.
Good job, this is an awesome first draft!
| The Parser feature is designed to be modular and extensible, allowing for easy addition of new commands and functionalities. The core components of the Parser feature include: | ||
| 1. **Command Classes**: Each command is represented by a separate class, which encapsulates the logic and parameters associated with that command. This design allows for clear separation of concerns and makes it easy to add new commands in the future. | ||
|
|
||
| 2. **Parser Class**: The `Parser` class is responsible for parsing the user input and instantiating the appropriate command class. It uses a series of `switch` statements to determine the type of command based on the input string. |
There was a problem hiding this comment.
it's better to explain how parser works based on the input
| ### Sequence Diagram | ||
| The following sequence diagram illustrates the interaction between the user, the Ingredient Inventory, and the | ||
| Ingredient feature during ingredient management: | ||
|
|
There was a problem hiding this comment.
It is better to explain how each command interact with other classes
| 3. **Command Execution**: Once the command is parsed and instantiated, it is executed by calling the `execute` method on the command object. This method contains the logic for performing the action associated with the command. | ||
|
|
||
| 4. **Error Handling**: The Parser feature includes error handling to manage invalid input and provide feedback to the user. If the input cannot be parsed or if the command is not recognized, an appropriate error message is displayed. | ||
|
|
| ## Sequence Diagram | ||
| The following sequence diagram illustrates the interaction between the user, the Ingredient Inventory, and the | ||
| Ingredient feature during ingredient management: | ||
|
|
There was a problem hiding this comment.
It is better to make the larger difference between all the feature titles, for now it is hard to read.
| The following sequence diagram illustrates the interaction between the user, the `Parser`, and the `Command` classes during the execution of a command: | ||
|
|
||
| <img src="img_1.png" alt="drawing" style="width:900px;"/> | ||
|
|
There was a problem hiding this comment.
For this diagram, here is no return 'dot' line and activation bar
| The following sequence diagram illustrates the interaction between the `Storage` class and the other key lists in the application during the loading and saving of data: | ||
| The key lists include: `StockList`, `LowStockList`, `RecipeList`, `ShoppingList` | ||
| <img src="img_3.png" alt="drawing" style="width:1000px;"/> | ||
|
|
There was a problem hiding this comment.
For the sequence diagram, the return 'dot' line is necessary, and it is better to regular the input format.
xmtan1
left a comment
xmtan1
left a comment
There was a problem hiding this comment.
Other than the long lines of text, I think this DG was quite comprehensive!
|
|
||
| # Class Diagram | ||
| <img src="img_2.png" alt="drawing" style="width:1500px;"/> | ||
|
|
There was a problem hiding this comment.
For the class diagram, I think it would be best to omit the names of the parameters in the class methods as the class diagram is currently quite small and hard to read..
e.g. +task(String name, Ingredient ingredient) --> +task(String, Ingredient)
| ## Instructions for manual testing | ||
|
|
||
| {Give instructions on how to do a manual product testing e.g., how to load sample data to be used for testing} | ||
|
|
There was a problem hiding this comment.
Maybe add a short description of what your program does?
| - **Add Plan**: Instantiates a new Meal Plan instance with a name optionally keyed by the user | ||
| - **Remove Plan**: Removes a plan from the currently created preset | ||
| - **Add recipe to plan**: Facilitates the procedure of adding a recipe to the indicated meal plan as decided by the user | ||
| - **Remove recipe from plan**: Facilitates the procedure of removing a recipe from the indicated meal plan as decided by the user |
There was a problem hiding this comment.
Would it be possible to add a diagram here to break up the long lines of text? For example for each feature I think it would be better to add a diagram for the core functionalities with boxes and arrows. Description of each functionality is self explanatory from the name and doesn't need to be explicity listed in my opinion.
| The following sequence diagram illustrates the interaction between the user, the Ingredient Inventory, and the | ||
| Ingredient feature during ingredient management: | ||
|
|
||
| <img src="img_4.png" alt="drawing" style="width:500px;"/> |
There was a problem hiding this comment.
For each method call, I think it would be good to add a dotted arrow back to caller class if the method returns something
| The Storage feature is designed to be modular and extensible, allowing for easy addition of new data types and functionalities. The core components of the Storage feature include: | ||
| 1. **Storage Class**: The `Storage` class is responsible for managing the reading and writing of data to and from `data.txt`. It provides methods for loading data from the file and saving data back to the file. | ||
| 2. **Data Classes**: The Storage feature interacts with various data classes, such as `IngredientInventory`, `ShoppingList`, `RecipeManager`, and `PlanPresets`. Each of these classes represents a specific type of data list and provides methods for manipulating that list. | ||
| 3. **File Handling**: The Storage feature uses file handling techniques to read and write data to `data.txt`. It ensures that the data is formatted correctly and that any errors during file operations are handled gracefully. |
There was a problem hiding this comment.
Could provide some explanation or example of error handling (e.g. throwing exception)?
| The Command classes are designed to be modular and expandable, allowing for easy addition of new commands and functionalities. | ||
| The core components of the Command classes include: | ||
| 1. **Command Classes**: Each command is represented by a separate class, which encapsulates the logic and parameters associated with that command. This design allows for clear separation of concerns and makes it easy to add new commands in the future. | ||
| 2. **Command Types**: Each command inherits from a specific command type, which is used to identify the command's functionality group. This allows for better organization and management of commands within the application. |
There was a problem hiding this comment.
Maybe the guide could clarify what these command types are (e.g. abstract classes/interfaces)? A brief list or example might help.
There was a problem hiding this comment.
Should the lifelines of the command objects (RecipeCommand, MealPlanCommand, etc.) continue after execute(...) to show that the object might still exist or interact with other components later?
There was a problem hiding this comment.
Should there be return arrows from the execute(...) methods back to the Parser or User lifelines? It might help show what kind of response or result is returned after command execution.
Add JUnit tests for Recipe, add Recipe section for User Guide
…into branch-enhancements
Add admin's tp requirements
Branch enhancements
Add defensiveness to commands
MealPlan commands naming refresh
* commit '9925f1542e215247092bce50aca65f469a2d46de': Rename function removePlanFromList to align with matching user input Fix: Gradle Coding Style Issues Add JavaDoc to Parser, Storage, UI Fix EXPECTED.txt Update for EXPECTED.txt Add JUnit Tests Update PantryPal.java: Main class is more OOP Add to commit for upstream pull Add on merge fixes Add bugfix attempts
Added Product Description in TP ReadME
Sequence diagram addition for MealPlan
Bug Fix: Add Meat into category Storage Bug Fix: Add exception handling for illegal arguments
Update UG: Add Meal Plan Commands
Update UG: update command summary and added missing findplan command
DG Update
Update PPP
added captions for meal plan sequence diagram to provide context
Update DeveloperGuide.md
Update PPP
Update PPP
update PPP
14 participants
PantryPal provides NUS students a better way to manage their meals by presenting all essential information at a glance, ensuring effortless meal planning via ingredient tracking and grocery management.