Skip to content

Commit

Permalink
Add cumulative invalid parameter errors
Browse files Browse the repository at this point in the history
When a user uses a command with multiple flags wrong, namely `add` and
`edit`, the error message only displays an error for the first invalid
flag.

Now the error message is cumulative, allowing users to correct multiple
wrong flag values with 1 error message.
  • Loading branch information
ZShunRen committed Oct 31, 2024
1 parent 546af09 commit 2ee69e5
Show file tree
Hide file tree
Showing 9 changed files with 177 additions and 67 deletions.
38 changes: 38 additions & 0 deletions src/main/java/seedu/address/logic/commands/CommandCommons.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
package seedu.address.logic.commands;

import java.util.Set;
import java.util.function.Supplier;

import seedu.address.logic.parser.exceptions.ParseException;
import seedu.address.model.person.Person;

/**
Expand All @@ -11,4 +15,38 @@ public final class CommandCommons {
public static final String DEFAULT_STATUS = "NONE";
public static final Person EMPTY_PERSON = null;

/**
* A functional interface similar to {@link Supplier}, but allows for a checked exception, {@code ParseException}
* to be thrown.
* @param <T> The return type of the method supplied by this supplier.
*/
@FunctionalInterface
public interface ThrowingSupplier<T> {
T get() throws ParseException;
}

/**
* Used to parse a field in a command, and if there are problems with the field, add the resulting error message
* to a {@link Set}, to be displayed to the user
* @param <T> The attribute type to be parsed
* @param parserMethod A lambda expression that contains a method, that when run can either return the parsed value
* or throws a {@code ParseException}
* @param errors A {@link Set} that contains the error messages
* @return The parsed value or null
*/
public static <T> T parseField(ThrowingSupplier<? extends T> parserMethod, Set<String> errors) {
try {
return parserMethod.get();
} catch (ParseException e) {
errors.add(e.getMessage());
return null;
}
}

public static String getErrorMessage(Set<String> errors) {
StringBuilder accumulatedErrors = new StringBuilder();
errors.forEach(error -> accumulatedErrors.append(error + "\n"));
return accumulatedErrors.toString().trim();
}

}
43 changes: 29 additions & 14 deletions src/main/java/seedu/address/logic/parser/AddCommandParser.java
Original file line number Diff line number Diff line change
@@ -1,17 +1,21 @@
package seedu.address.logic.parser;

import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.address.logic.commands.CommandCommons.getErrorMessage;
import static seedu.address.logic.commands.CommandCommons.parseField;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
import static seedu.address.logic.parser.CliSyntax.PREFIX_INCOME;
import static seedu.address.logic.parser.CliSyntax.PREFIX_JOB;
import static seedu.address.logic.parser.CliSyntax.PREFIX_NAME;
import static seedu.address.logic.parser.CliSyntax.PREFIX_NEW_REMARK;
import static seedu.address.logic.parser.CliSyntax.PREFIX_PHONE;
import static seedu.address.logic.parser.CliSyntax.PREFIX_REMARK;
import static seedu.address.logic.parser.CliSyntax.PREFIX_STATUS;
import static seedu.address.logic.parser.CliSyntax.PREFIX_TIER;

import java.util.LinkedHashSet;
import java.util.List;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

Expand All @@ -35,6 +39,7 @@
*/
public class AddCommandParser implements Parser<AddCommand> {


/**
* Parses the given {@code String} of arguments in the context of the AddCommand
* and returns an AddCommand object for execution.
Expand All @@ -44,7 +49,7 @@ public class AddCommandParser implements Parser<AddCommand> {
public AddCommand parse(String args) throws ParseException {
ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_JOB,
PREFIX_INCOME, PREFIX_TIER, PREFIX_NEW_REMARK, PREFIX_STATUS);
PREFIX_INCOME, PREFIX_TIER, PREFIX_REMARK, PREFIX_STATUS);
PrefixCheckResult prefixCheckResult = arePrefixesPresent(argMultimap, PREFIX_NAME, PREFIX_ADDRESS,
PREFIX_JOB, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_INCOME);
if (!prefixCheckResult.isAllPrefixPresent()) {
Expand All @@ -56,22 +61,32 @@ public AddCommand parse(String args) throws ParseException {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE));
}
argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_JOB,
PREFIX_INCOME, PREFIX_TIER, PREFIX_NEW_REMARK, PREFIX_STATUS);
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get());
Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get());
Email email = ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get());
Address address = ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get());
Job job = ParserUtil.parseJob(argMultimap.getValue(PREFIX_JOB).get());
Income income = ParserUtil.parseIncome(argMultimap.getValue(PREFIX_INCOME).get());
Tier tier = ParserUtil.parseTier(argMultimap.getValue(PREFIX_TIER).orElse(CommandCommons.DEFAULT_TIER));
Remark remark = ParserUtil.parseNewRemark(argMultimap.getValue(PREFIX_NEW_REMARK)
.orElse(CommandCommons.DEFAULT_REMARK));
Status status =
ParserUtil.parseStatus(argMultimap.getValue(PREFIX_STATUS).orElse(CommandCommons.DEFAULT_STATUS));
PREFIX_INCOME, PREFIX_TIER, PREFIX_REMARK, PREFIX_STATUS);
Set<String> errors = new LinkedHashSet<>();
Name name = parseField(() -> ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get()), errors);
Phone phone = parseField(() -> ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get()), errors);
Email email = parseField(() -> ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get()), errors);
Address address = parseField(() -> ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get()),
errors);
Job job = parseField(() -> ParserUtil.parseJob(argMultimap.getValue(PREFIX_JOB).get()), errors);
Income income = parseField(() -> ParserUtil.parseIncome(argMultimap.getValue(PREFIX_INCOME).get()),
errors);
Tier tier = parseField(() -> ParserUtil.parseTier(argMultimap.getValue(PREFIX_TIER)
.orElse(CommandCommons.DEFAULT_TIER)), errors);
Remark remark = parseField(() -> ParserUtil.parseRemark(argMultimap.getValue(PREFIX_REMARK)
.orElse(CommandCommons.DEFAULT_REMARK)), errors);
Status status = parseField(() -> ParserUtil.parseStatus(argMultimap.getValue(PREFIX_STATUS)
.orElse(CommandCommons.DEFAULT_STATUS)), errors);
if (!errors.isEmpty()) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT,
getErrorMessage(errors) + "\n" + AddCommand.MESSAGE_USAGE));
}
Person person = new Person(name, phone, email, address, job, income, tier, remark, status);
return new AddCommand(person);
}



/**
* Returns {@code PrefixCheckResult} if none of the prefixes contains empty {@code Optional} values in the given
* {@code ArgumentMultimap}.
Expand Down
45 changes: 32 additions & 13 deletions src/main/java/seedu/address/logic/parser/EditCommandParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

import static java.util.Objects.requireNonNull;
import static seedu.address.logic.Messages.MESSAGE_INVALID_COMMAND_FORMAT;
import static seedu.address.logic.commands.CommandCommons.getErrorMessage;
import static seedu.address.logic.commands.CommandCommons.parseField;
import static seedu.address.logic.parser.CliSyntax.PREFIX_ADDRESS;
import static seedu.address.logic.parser.CliSyntax.PREFIX_APPEND_REMARK;
import static seedu.address.logic.parser.CliSyntax.PREFIX_EMAIL;
Expand All @@ -13,6 +15,9 @@
import static seedu.address.logic.parser.CliSyntax.PREFIX_STATUS;
import static seedu.address.logic.parser.CliSyntax.PREFIX_TIER;

import java.util.LinkedHashSet;
import java.util.Set;

import seedu.address.commons.core.index.Index;
import seedu.address.logic.Messages;
import seedu.address.logic.commands.EditCommand;
Expand Down Expand Up @@ -53,38 +58,52 @@ public EditCommand parse(String args) throws ParseException {
PREFIX_INCOME, PREFIX_JOB, PREFIX_TIER, PREFIX_NEW_REMARK, PREFIX_APPEND_REMARK, PREFIX_STATUS);

EditPersonDescriptor editPersonDescriptor = new EditPersonDescriptor();

Set<String> errors = new LinkedHashSet<>();
if (argMultimap.getValue(PREFIX_NAME).isPresent()) {
editPersonDescriptor.setName(ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get()));
editPersonDescriptor.setName(
parseField(() -> ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).get()), errors));
}
if (argMultimap.getValue(PREFIX_PHONE).isPresent()) {
editPersonDescriptor.setPhone(ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get()));
editPersonDescriptor.setPhone(
parseField(() -> ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).get()), errors));
}
if (argMultimap.getValue(PREFIX_EMAIL).isPresent()) {
editPersonDescriptor.setEmail(ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get()));
editPersonDescriptor.setEmail(
parseField(() -> ParserUtil.parseEmail(argMultimap.getValue(PREFIX_EMAIL).get()), errors));
}
if (argMultimap.getValue(PREFIX_ADDRESS).isPresent()) {
editPersonDescriptor.setAddress(ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get()));
editPersonDescriptor.setAddress(
parseField(() -> ParserUtil.parseAddress(argMultimap.getValue(PREFIX_ADDRESS).get()),
errors));
}
if (argMultimap.getValue(PREFIX_JOB).isPresent()) {
editPersonDescriptor.setJob(ParserUtil.parseJob(argMultimap.getValue(PREFIX_JOB).get()));
editPersonDescriptor.setJob(parseField(() -> ParserUtil.parseJob(argMultimap.getValue(PREFIX_JOB).get()),
errors));
}
if (argMultimap.getValue(PREFIX_INCOME).isPresent()) {
editPersonDescriptor.setIncome(ParserUtil.parseIncome(argMultimap.getValue(PREFIX_INCOME).get()));
editPersonDescriptor.setIncome(parseField(() -> ParserUtil.parseIncome(
argMultimap.getValue(PREFIX_INCOME).get()), errors));
}
if (argMultimap.getValue(PREFIX_TIER).isPresent()) {
editPersonDescriptor.setTier(ParserUtil.parseTier(argMultimap.getValue(PREFIX_TIER).get()));
editPersonDescriptor.setTier(parseField(() -> ParserUtil.parseTier(
argMultimap.getValue(PREFIX_TIER).get()), errors));
}
if (argMultimap.getValue(PREFIX_NEW_REMARK).isPresent()) {
editPersonDescriptor.setNewRemark(ParserUtil.parseNewRemark(argMultimap.getValue(PREFIX_NEW_REMARK).get()));
editPersonDescriptor.setNewRemark(parseField(() -> ParserUtil.parseNewRemark(
argMultimap.getValue(PREFIX_NEW_REMARK).get()), errors));
}
if (argMultimap.getValue(PREFIX_APPEND_REMARK).isPresent()) {
editPersonDescriptor.setAppendedRemark(ParserUtil.parseNewRemark(
argMultimap.getValue(PREFIX_APPEND_REMARK).get()));
editPersonDescriptor.setAppendedRemark(parseField(() -> ParserUtil.parseNewRemark(
argMultimap.getValue(PREFIX_APPEND_REMARK).get()), errors));
}
if (argMultimap.getValue(PREFIX_STATUS).isPresent()) {
editPersonDescriptor.setStatus((ParserUtil.parseStatus(
argMultimap.getValue(PREFIX_STATUS).get())));
editPersonDescriptor.setStatus(parseField(() -> ParserUtil.parseStatus(
argMultimap.getValue(PREFIX_STATUS).get()), errors));
}

if (!errors.isEmpty()) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT,
getErrorMessage(errors)));
}

if (!editPersonDescriptor.isAnyFieldEdited()) {
Expand Down
1 change: 0 additions & 1 deletion src/main/java/seedu/address/logic/parser/ParserUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,6 @@ public static IncomeComparisonOperator parseIncomeComparisonOperator(String oper
requireNonNull(operator);
String trimmedOperator = operator.trim();
if (!IncomeComparisonOperator.isValidComparisonOperator(trimmedOperator)) {
System.out.println(("HERE"));
throw new ParseException(IncomeComparisonOperator.MESSAGE_CONSTRAINTS);
}
return new IncomeComparisonOperator(trimmedOperator);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ public class CommandTestUtil {
public static final String VALID_REMARK_BOB = "Favourite pastime: Eating";
public static final String TIER_DESC_REJECT = " " + PREFIX_TIER + VALID_TIER_REJECT;
public static final String TIER_DESC_GOLD = " " + PREFIX_TIER + VALID_TIER_GOLD;
public static final String REMARK_DESC_BOB = " " + PREFIX_REMARK + VALID_REMARK_BOB;
public static final String NEW_REMARK_DESC_BOB = " " + PREFIX_NEW_REMARK + VALID_REMARK_BOB;
public static final String APPEND_REMARK_DESC_BOB = " " + PREFIX_APPEND_REMARK + VALID_REMARK_BOB;
public static final String VALID_STATUS_NON_URGENT = "non_urgent";
Expand All @@ -79,7 +80,7 @@ public class CommandTestUtil {
public static final String INVALID_INCOME_DESC = " " + PREFIX_INCOME + "-999"; // negative numbers should not be
// allowed
public static final String INVALID_TIER_DESC = " " + PREFIX_TIER + "platinum"; // not one of the 4 TierEnums
public static final String INVALID_REMARK_DESC = " " + PREFIX_REMARK;
public static final String INVALID_REMARK_DESC = " " + PREFIX_REMARK + "漢";
public static final String INVALID_NEW_REMARK_DESC = " " + PREFIX_NEW_REMARK;
public static final String INVALID_APPEND_REMARK_DESC = " " + PREFIX_APPEND_REMARK;
public static final String INVALID_STATUS_DESC = " " + PREFIX_STATUS + "null";
Expand Down
Loading

0 comments on commit 2ee69e5

Please sign in to comment.