Skip to content
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 Delete Command #93

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/logic/Messages.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ public class Messages {

public static final String MESSAGE_UNKNOWN_COMMAND = "Unknown command";
public static final String MESSAGE_INVALID_COMMAND_FORMAT = "Invalid command format! \n%1$s";
public static final String MESSAGE_INVALID_INGREDIENT_DISPLAYED_INDEX = "The ingredient index provided is invalid";
public static final String MESSAGE_INVALID_RECIPE_DISPLAYED_INDEX = "The recipe index provided is invalid";
public static final String MESSAGE_INGREDIENTS_LISTED_OVERVIEW = "%1$d ingredients listed!";
public static final String MESSAGE_DUPLICATE_FIELDS =
"Multiple values specified for the following single-valued field(s): ";
Expand Down
142 changes: 72 additions & 70 deletions src/main/java/seedu/address/logic/commands/DeleteCommand.java
Original file line number Diff line number Diff line change
@@ -1,70 +1,72 @@
//package seedu.address.logic.commands;
//
//import static java.util.Objects.requireNonNull;
//
//import java.util.List;
//
//import seedu.address.commons.core.index.Index;
//import seedu.address.commons.util.ToStringBuilder;
//import seedu.address.logic.Messages;
//import seedu.address.logic.commands.exceptions.CommandException;
//import seedu.address.model.Model;
//import seedu.address.model.ingredient.Ingredient;
//
///**
// * Deletes a ingredient identified using it's displayed index from the inventory.
// */
//public class DeleteCommand extends Command {
//
// public static final String COMMAND_WORD = "delete";
//
// public static final String MESSAGE_USAGE = COMMAND_WORD
// + ": Deletes the ingredient identified by the index number used in the displayed ingredient list.\n"
// + "Parameters: INDEX (must be a positive integer)\n"
// + "Example: " + COMMAND_WORD + " 1";
//
// public static final String MESSAGE_DELETE_INGREDIENT_SUCCESS = "Deleted Ingredient: %1$s";
//
// private final Index targetIndex;
//
// public DeleteCommand(Index targetIndex) {
// this.targetIndex = targetIndex;
// }
//
// @Override
// public CommandResult execute(Model model) throws CommandException {
// requireNonNull(model);
// List<Ingredient> lastShownList = model.getFilteredIngredientList();
//
// if (targetIndex.getZeroBased() >= lastShownList.size()) {
// throw new CommandException(Messages.MESSAGE_INVALID_INGREDIENT_DISPLAYED_INDEX);
// }
//
// Ingredient ingredientToDelete = lastShownList.get(targetIndex.getZeroBased());
// model.deleteIngredient(ingredientToDelete);
// return new CommandResult(String.format(MESSAGE_DELETE_INGREDIENT_SUCCESS,
// Messages.format(ingredientToDelete)));
// }
//
// @Override
// public boolean equals(Object other) {
// if (other == this) {
// return true;
// }
//
// // instanceof handles nulls
// if (!(other instanceof DeleteCommand)) {
// return false;
// }
//
// DeleteCommand otherDeleteCommand = (DeleteCommand) other;
// return targetIndex.equals(otherDeleteCommand.targetIndex);
// }
//
// @Override
// public String toString() {
// return new ToStringBuilder(this)
// .add("targetIndex", targetIndex)
// .toString();
// }
//}
package seedu.address.logic.commands;

import static java.util.Objects.requireNonNull;

import java.util.List;

import seedu.address.commons.core.index.Index;
import seedu.address.commons.util.ToStringBuilder;
import seedu.address.logic.Messages;
import seedu.address.logic.commands.exceptions.CommandException;
import seedu.address.model.Model;
import seedu.address.model.recipe.Recipe;

/**
* Deletes a recipe identified using it's displayed index from the recipe book.
*/
public class DeleteCommand extends Command {

public static final String COMMAND_WORD = "delete";

public static final String MESSAGE_USAGE = COMMAND_WORD
+ ": Deletes the recipe identified by the index number used in the displayed recipe list.\n"
+ "Parameters: INDEX (must be a positive integer)\n"
+ "Example: " + COMMAND_WORD + " 1";

public static final String MESSAGE_DELETE_RECIPE_SUCCESS = "Deleted Recipe: %1$s";

private final Index targetIndex;

public DeleteCommand(Index targetIndex) {
this.targetIndex = targetIndex;
}

@Override
public CommandResult execute(Model model) throws CommandException {
requireNonNull(model);
List<Recipe> lastShownList = model.getFilteredRecipeList();

if (targetIndex.getOneBased() > lastShownList.size() || targetIndex.getOneBased() <= 0) {
throw new CommandException(Messages.MESSAGE_INVALID_RECIPE_DISPLAYED_INDEX);
}
Recipe deletedRecipe = lastShownList.get(targetIndex.getZeroBased());
model.deleteRecipe(deletedRecipe);

return new CommandResult(String.format(MESSAGE_DELETE_RECIPE_SUCCESS,
targetIndex.getOneBased()));


}

@Override
public boolean equals(Object other) {
if (other == this) {
return true;
}

// instanceof handles nulls
if (!(other instanceof DeleteCommand)) {
return false;
}

DeleteCommand otherDeleteCommand = (DeleteCommand) other;
return (targetIndex.equals(otherDeleteCommand.targetIndex));
}

@Override
public String toString() {
return new ToStringBuilder(this)
.add("targetIndex", targetIndex)
.toString();
}
}
58 changes: 29 additions & 29 deletions src/main/java/seedu/address/logic/parser/DeleteCommandParser.java
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
//package seedu.address.logic.parser;
//
//import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
//
//import seedu.address.commons.core.index.Index;
//import seedu.address.logic.commands.DeleteCommand;
//import seedu.address.logic.parser.exceptions.ParseException;
//
///**
// * Parses input arguments and creates a new DeleteCommand object
// */
//public class DeleteCommandParser implements Parser<DeleteCommand> {
//
// /**
// * Parses the given {@code String} of arguments in the context of the DeleteCommand
// * and returns a DeleteCommand object for execution.
// * @throws ParseException if the user input does not conform the expected format
// */
// public DeleteCommand parse(String args) throws ParseException {
// try {
// Index index = ParserUtil.parseIndex(args);
// return new DeleteCommand(index);
// } catch (ParseException pe) {
// throw new ParseException(
// String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), pe);
// }
// }
//
//}
package seedu.address.logic.parser;
import seedu.address.commons.core.index.Index;
import seedu.address.logic.commands.DeleteCommand;
import seedu.address.logic.parser.exceptions.ParseException;

import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;

/**
* Parses input arguments and creates a new DeleteCommand object
*/
public class DeleteCommandParser implements Parser<DeleteCommand> {

/**
* Parses the given {@code String} of arguments in the context of the DeleteCommand
* and returns a DeleteCommand object for execution.
* @throws ParseException if the user input does not conform the expected format
*/
public DeleteCommand parse(String args) throws ParseException {
try {
String trimmedArgs = args.trim();
Index index = ParserUtil.parseIndex(trimmedArgs);
return new DeleteCommand(index);
} catch (ParseException pe) {
throw new ParseException(
String.format(MESSAGE_INVALID_COMMAND_FORMAT, DeleteCommand.MESSAGE_USAGE), pe);
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import seedu.address.logic.commands.AddCommand;
import seedu.address.logic.commands.ClearCommand;
import seedu.address.logic.commands.Command;
import seedu.address.logic.commands.DeleteCommand;
import seedu.address.logic.commands.ExitCommand;
import seedu.address.logic.commands.HelpCommand;
import seedu.address.logic.commands.ListCommand;
Expand Down Expand Up @@ -77,6 +78,9 @@ public Command parseCommand(String userInput) throws ParseException {
case RecipeViewCommand.COMMAND_WORD:
return new RecipeViewCommandParser().parse(arguments);

case DeleteCommand.COMMAND_WORD:
return new DeleteCommandParser().parse(arguments);

default:
logger.finer("This user input caused a ParseException: " + userInput);
throw new ParseException(MESSAGE_UNKNOWN_COMMAND);
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/seedu/address/model/Model.java
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public interface Model {

boolean hasRecipe(Name recipeName);

void deleteRecipe(int recipeId);
void deleteRecipe(Recipe recipe);

void addRecipe(Recipe recipe);

Expand Down
5 changes: 2 additions & 3 deletions src/main/java/seedu/address/model/ModelManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,9 @@ public boolean hasRecipe(Name recipeName) {
}

@Override
public void deleteRecipe(int recipeId) {
this.recipeBook.removeRecipe(recipeId);
public void deleteRecipe(Recipe recipe) {
this.recipeBook.removeRecipe(recipe);
}

@Override
public void addRecipe(Recipe recipe) {
requireNonNull(recipe);
Expand Down
14 changes: 6 additions & 8 deletions src/main/java/seedu/address/model/RecipeBook.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,14 @@ public void addRecipe(Recipe toAdd) {
}

/**
* Removes a recipe from the recipe book with its {@code recipeId}
* Removes a recipe from the recipe book
*/
public void removeRecipe(int recipeId) {
for (Recipe recipe : this.recipeList) {
if (recipe.getId() == recipeId) {
this.recipeList.remove(recipe);
return;
}
public void removeRecipe(Recipe recipe) {
try {
this.recipeList.remove(recipe);
} catch (RecipeNotFoundException e) {
throw new RecipeNotFoundException();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think throwing an exception is fine so that the code can catch and display the corresponding error if needed

}
throw new RecipeNotFoundException();
}

/**
Expand Down
27 changes: 14 additions & 13 deletions src/test/java/seedu/address/logic/commands/DeleteCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
//import seedu.address.logic.Messages;
//import seedu.address.model.Model;
//import seedu.address.model.ModelManager;
//import seedu.address.model.RecipeBook;
//import seedu.address.model.UserPrefs;
//import seedu.address.model.ingredient.Ingredient;
//
Expand All @@ -25,21 +26,21 @@
// */
//public class DeleteCommandTest {
//
// private Model model = new ModelManager(getTypicalInventory(), new UserPrefs());
// private Model model = new ModelManager(getTypicalInventory(), new UserPrefs(), new RecipeBook());
//
//// @Test
//// public void execute_validIndexUnfilteredList_success() {
// @Test
// public void execute_validIndexUnfilteredList_success() {
// Ingredient ingredientToDelete = model.getFilteredIngredientList().get(INDEX_FIRST_INGREDIENT.getZeroBased());
//// DeleteCommand deleteCommand = new DeleteCommand(INDEX_FIRST_INGREDIENT);
////
//// String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_INGREDIENT_SUCCESS,
//// Messages.format(ingredientToDelete));
////
//// ModelManager expectedModel = new ModelManager(model.getInventory(), new UserPrefs());
//// expectedModel.deleteIngredient(ingredientToDelete);
////
//// assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModel);
//// }
// DeleteCommand deleteCommand = new DeleteCommand(INDEX_FIRST_INGREDIENT);
//
// String expectedMessage = String.format(DeleteCommand.MESSAGE_DELETE_INGREDIENT_SUCCESS,
// Messages.format(ingredientToDelete));
//
// ModelManager expectedModel = new ModelManager(model.getInventory(), new UserPrefs());
// expectedModel.deleteIngredient(ingredientToDelete);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

//
// assertCommandSuccess(deleteCommand, model, expectedMessage, expectedModel);
// }
//
//// @Test
//// public void execute_invalidIndexUnfilteredList_throwsCommandException() {
Expand Down
24 changes: 12 additions & 12 deletions src/test/java/seedu/address/model/RecipeBookTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,18 @@ public void hasRecipe_recipeNotInRecipeBook_returnsFalse() {
assertFalse(recipeBook.hasRecipe(SPONGECAKE.getName()));
}

@Test
public void removeRecipe_recipeInRecipeBook_success() {
recipeBook.addRecipe(COOKIES);
recipeBook.removeRecipe(COOKIES.getId());
assertEquals(new RecipeBook(), recipeBook);
}

@Test
public void removeRecipe_recipeNotInRecipeBook_throwsRecipeNotFoundException() {
recipeBook.addRecipe(COOKIES);
assertThrows(RecipeNotFoundException.class, () -> recipeBook.removeRecipe(SPONGECAKE.getId()));
}
// @Test
// public void removeRecipe_recipeInRecipeBook_success() {
// recipeBook.addRecipe(COOKIES);
// recipeBook.removeRecipe(COOKIES.getId());
// assertEquals(new RecipeBook(), recipeBook);
// }
//
// @Test
// public void removeRecipe_recipeNotInRecipeBook_throwsRecipeNotFoundException() {
// recipeBook.addRecipe(COOKIES);
// assertThrows(RecipeNotFoundException.class, () -> recipeBook.removeRecipe(SPONGECAKE.getId()));
// }
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests could be ammended by removing the getId() method call


@Test
public void clear_recipeInRecipeBook_returnsEmptyList() {
Expand Down