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

Refactor command error messages #148

Merged
merged 7 commits into from
Nov 2, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
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
14 changes: 13 additions & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ task coverage(type: JacocoReport) {
}

dependencies {
// classpath "com.github.johnrengelman.shadow:7.1.2"
String jUnitVersion = '5.4.0'
String javaFxVersion = '17.0.7'

Expand Down Expand Up @@ -79,6 +80,17 @@ dependencies {

shadowJar {
archiveFileName = 'AgentAssist.jar'
zip64 = true
minimize()
exclude('**/META-INF/*.SF', '**/META-INF/*.DSA', '**/META-INF/*.RSA')
exclude('**/*.md') // Markdown files
exclude('**/*.txt') // Text files
// exclude('**/*.png', '**/*.jpg') // Images, if not required

Choose a reason for hiding this comment

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

don't forget to clean up this commented lines later!

exclude('**/docs/**')
exclude('**/src/test/**')
}
task analyzeJar(type: Copy) {
from(zipTree(file('build/libs/AgentAssist.jar')))
into 'build/analyzedJar'
}

defaultTasks 'clean', 'test'
32 changes: 32 additions & 0 deletions src/main/java/seedu/address/commons/util/PrefixCheckResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package seedu.address.commons.util;

import java.util.List;

import seedu.address.logic.parser.Prefix;

/**
* Contains the results of checking for missing prefixes.
*/
public class PrefixCheckResult {
private final boolean isAllPrefixPresent;
private final List<Prefix> missingPrefixes;

/**
* Constructs a {@code PrefixCheckResult} object.
* @param isAllPrefixPresent boolean value that is true when all mandatory prefixes are present.
* @param missingPrefixes a {@code List} object that contains all the missing prefixes in the invalid add command.
*/
public PrefixCheckResult(boolean isAllPrefixPresent, List<Prefix> missingPrefixes) {
this.isAllPrefixPresent = isAllPrefixPresent;
this.missingPrefixes = missingPrefixes;
}

public boolean isAllPrefixPresent() {
return this.isAllPrefixPresent;
}

public List<Prefix> getMissingPrefixes() {
return this.missingPrefixes;
}

}
31 changes: 18 additions & 13 deletions src/main/java/seedu/address/logic/commands/AddCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,22 @@ public class AddCommand extends Command {

public static final String COMMAND_WORD = "add";

public static final String MESSAGE_USAGE = COMMAND_WORD + ": Adds a person to the address book. "
+ "Parameters: "
+ PREFIX_NAME + "NAME "
+ PREFIX_PHONE + "PHONE "
+ PREFIX_EMAIL + "EMAIL "
+ PREFIX_ADDRESS + "ADDRESS "
+ PREFIX_JOB + "JOB "
+ PREFIX_INCOME + "INCOME "
+ "[" + PREFIX_TIER + "TIER]...\n"
+ "[" + PREFIX_NEW_REMARK + "NEW REMARK]..."
+ "[" + PREFIX_STATUS + "STATUS]...\n"
+ "Example: " + COMMAND_WORD + " "
public static final String MISSING_PREFIX_MESSAGE_START = "The following mandatory prefixes are missing: ";

public static final String MESSAGE_USAGE = COMMAND_WORD + ": Adds a person to the address book.\n"
+ "Required Parameters: "
+ PREFIX_NAME + " NAME "
+ PREFIX_PHONE + " PHONE "
+ PREFIX_EMAIL + " EMAIL "
+ PREFIX_ADDRESS + " ADDRESS "
+ PREFIX_JOB + " JOB "
+ PREFIX_INCOME + " INCOME\n"
+ "Optional Parameters: "
+ "[" + PREFIX_TIER + " TIER] "
+ "[" + PREFIX_NEW_REMARK + " NEW REMARK] "
+ "[" + PREFIX_STATUS + " STATUS] \n"
+ "Example Usage: '"
+ COMMAND_WORD + " "
+ PREFIX_NAME + "John Doe "
+ PREFIX_PHONE + "98765432 "
+ PREFIX_EMAIL + "[email protected] "
Expand All @@ -44,7 +48,8 @@ public class AddCommand extends Command {
+ PREFIX_INCOME + "300 "
+ PREFIX_TIER + "GOLD "
+ PREFIX_NEW_REMARK + "He is very smart "
+ PREFIX_STATUS + "NON_URGENT";
+ PREFIX_STATUS + "NON_URGENT"
+ "'";

public static final String MESSAGE_SUCCESS = "New person added: %1$s";
public static final String MESSAGE_DUPLICATE_PERSON = "This person already exists in the address book";
Expand Down
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> {

Choose a reason for hiding this comment

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

Nice abstraction with ThrowingSupplier!

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();
}

}
27 changes: 14 additions & 13 deletions src/main/java/seedu/address/logic/commands/EditCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,21 @@ public class EditCommand extends Command {
public static final String MESSAGE_USAGE = COMMAND_WORD + ": Edits the details of the person identified "
+ "by the index number used in the displayed person list. "
+ "Existing values will be overwritten by the input values. Any fields unspecified will not be modified.\n"
+ "Parameters: INDEX (must be a positive integer) "
+ "[" + PREFIX_NAME + "NAME] "
+ "[" + PREFIX_PHONE + "PHONE] "
+ "[" + PREFIX_EMAIL + "EMAIL] "
+ "[" + PREFIX_ADDRESS + "ADDRESS] "
+ "[" + PREFIX_JOB + "JOB] "
+ "[" + PREFIX_INCOME + "INCOME] "
+ "[" + PREFIX_TIER + "TIER]...\n"
+ "[" + PREFIX_NEW_REMARK + "NEW REMARK] "
+ "[" + PREFIX_APPEND_REMARK + "ADD-ON TO EXISTING REMARK] "
+ "[" + PREFIX_STATUS + "STATUS] "
+ "Example: " + COMMAND_WORD + " 1 "
+ "Required Parameters: INDEX (must be a positive integer)\n"
+ "Optional Parameters: "
+ "[" + PREFIX_NAME + " NAME] "
+ "[" + PREFIX_PHONE + " PHONE] "
+ "[" + PREFIX_EMAIL + " EMAIL] "
+ "[" + PREFIX_ADDRESS + " ADDRESS] "
+ "[" + PREFIX_JOB + " JOB] "
+ "[" + PREFIX_INCOME + " INCOME] "
+ "[" + PREFIX_TIER + " TIER] "
+ "[" + PREFIX_NEW_REMARK + " NEW REMARK] "
+ "[" + PREFIX_APPEND_REMARK + " ADD-ON TO EXISTING REMARK] "
+ "[" + PREFIX_STATUS + " STATUS] \n"
+ "Example Usage: '" + COMMAND_WORD + " 1 "
+ PREFIX_PHONE + "91234567 "
+ PREFIX_EMAIL + "[email protected]";
+ PREFIX_EMAIL + "[email protected]'";

public static final String MESSAGE_EDIT_PERSON_SUCCESS = "Edited Person: %1$s";
public static final String MESSAGE_NOT_EDITED = "At least one field to edit must be provided.";
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/seedu/address/logic/commands/FilterCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ public class FilterCommand extends Command {
public static final String MESSAGE_USAGE = COMMAND_WORD + ": Searches for all customers whose specified field "
+ "contains the given substring (case-insensitive) and displays the results in a numbered list.\n"
+ "Parameters: <FLAG>/ <SEARCH TERM>\n"
+ "Flags: n/ (name), p/ (phone), e/ (email), a/ (address), j/ (job), r/ (remarks)\n"
+ "Example: " + COMMAND_WORD + " n/ Alice" + " p/ 91112222\n"
+ "Flags: n/ NAME, p/ PHONE, e/ EMAIL, a/ ADDRESS, j/ JOB, r/ REMARK\n"
+ "Example: '" + COMMAND_WORD + " n/ Alice" + " p/ 91112222'\n"
+ "This will find all customers whose names contain 'Alice' and whose phone number is '91112222'.";

private final Predicate<Person> predicate;
Expand Down
69 changes: 47 additions & 22 deletions src/main/java/seedu/address/logic/parser/AddCommandParser.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
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;

import seedu.address.commons.util.PrefixCheckResult;
import seedu.address.logic.commands.AddCommand;
import seedu.address.logic.commands.CommandCommons;
import seedu.address.logic.parser.exceptions.ParseException;
Expand All @@ -32,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 @@ -41,37 +49,54 @@ 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);

if (!arePrefixesPresent(argMultimap, PREFIX_NAME, PREFIX_ADDRESS, PREFIX_JOB, PREFIX_PHONE, PREFIX_EMAIL,
PREFIX_INCOME)
|| !argMultimap.getPreamble().isEmpty()) {
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()) {
String missingPrefixMessage = AddCommand.MISSING_PREFIX_MESSAGE_START
+ prefixCheckResult.getMissingPrefixes();
throw new ParseException(missingPrefixMessage + "\n" + String.format(MESSAGE_INVALID_COMMAND_FORMAT,
AddCommand.MESSAGE_USAGE));
} else if (!argMultimap.getPreamble().isEmpty()) {
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 true if none of the prefixes contains empty {@code Optional} values in the given
* Returns {@code PrefixCheckResult} if none of the prefixes contains empty {@code Optional} values in the given
* {@code ArgumentMultimap}.
*/
private static boolean arePrefixesPresent(ArgumentMultimap argumentMultimap, Prefix... prefixes) {
return Stream.of(prefixes).allMatch(prefix -> argumentMultimap.getValue(prefix).isPresent());
private static PrefixCheckResult arePrefixesPresent(ArgumentMultimap argumentMultimap, Prefix... prefixes) {
List<Prefix> listOfMissingPrefixes = Stream.of(prefixes)
.filter(prefix -> !argumentMultimap.getValue(prefix).isPresent())
.collect(Collectors.toList());
boolean isAllPrefixPresent = listOfMissingPrefixes.isEmpty();
return new PrefixCheckResult(isAllPrefixPresent, listOfMissingPrefixes);
}

}
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
Loading