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

Improve code quality #304

Merged
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 0 additions & 2 deletions src/main/java/seedu/address/logic/messages/EditMessages.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ public class EditMessages extends Messages {
+ "Edit requires a field prefix. %1$s \uD83D\uDC3E";
public static final String MESSAGE_EDIT_MISSING_NAME = "Failed to edit Pooch Contact - "
+ "Edit requires a name field. %1$s \uD83D\uDC3E";

public static final String MESSAGE_EDIT_NAME = "Failed to edit Pooch Contact - "
+ "Edit detect duplicate name fields. %1$s \uD83D\uDC3E";
public static final String MESSAGE_EDIT_INVALID_NAME = "Failed to edit Pooch Contact. %1$s \uD83D\uDC3E";
Expand All @@ -29,5 +28,4 @@ public class EditMessages extends Messages {
+ " Make sure that you are attempting to edit MAINTAINER.";
public static final String MESSAGE_INVALID_EDIT_SUPPLIER = "Name does not exist in our address book \uD83D\uDC3E"
+ " Make sure that you are attempting to edit SUPPLIER.";

}
4 changes: 2 additions & 2 deletions src/main/java/seedu/address/logic/messages/PinMessages.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
/**
* Container for user pin command visible messages.
*/
public class PinMessages {
public class PinMessages extends Messages {
public static final String MESSAGE_PIN_PERSON_SUCCESS = "Woof! Pinned %1$s successfully! \uD83D\uDC36";
public static final String MESSAGE_PIN_INVALID_NAME = "Failed to pin Pooch Contact - %1$s \uD83D\uDC3E";
public static final String MESSAGE_PIN_INVALID_NAME = " %1$s \uD83D\uDC3E";
public static final String MESSAGE_PIN_NAME_NOT_FOUND = "Failed to pin Pooch Contact - "
+ "Name does not exist in our address book \uD83D\uDC3E";
public static final String MESSAGE_PIN_MISSING_NAME = "Failed to pin Pooch Contact - "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
/**
* Container for user delete command visible messages.
*/
public class RateMessages {
public class RateMessages extends Messages {
public static final String MESSAGE_RATE_PERSON_SUCCESS = "Woof! Rated %1$s successfully! \uD83D\uDC36";
public static final String MESSAGE_RATE_NAME_NOT_FOUND = "Failed to rate Pooch Contact - "
+ "Name does not exist in our address book.\uD83D\uDC3E";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
/**
* Container for user unpin command visible messages.
*/
public class UnpinMessages {
public class UnpinMessages extends Messages {
public static final String MESSAGE_UNPIN_PERSON_SUCCESS = "Woof! Unpinned %1$s successfully! \uD83D\uDC36";
public static final String MESSAGE_UNPIN_INVALID_NAME = "Failed to unpin Pooch Contact - %1$s \uD83D\uDC3E";
public static final String MESSAGE_UNPIN_NAME_NOT_FOUND = "Failed to unpin Pooch Contact - "
Expand Down
30 changes: 21 additions & 9 deletions src/main/java/seedu/address/logic/parser/AddCommandParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,30 +35,44 @@ 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.
* and returns an AddCommand object for execution. Parameter args cannot be null.

Choose a reason for hiding this comment

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

should be @{code args} instead of 'args'

* @throws ParseException if the user input does not conform the expected format

Choose a reason for hiding this comment

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

Grammar error, missing preposition "to" between "conform" and "the".

*/
public AddCommand parse(String args) throws ParseException {
assert (args != null) : "`argument` to pass for add command is null";

logger.log(Level.INFO, "Going to start parsing for add command.");
ParserUtil.verifyNoUnknownPrefix(args, AddCommand.MESSAGE_USAGE, "add",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_NOTE, PREFIX_RATING);

ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_NOTE,
PREFIX_RATING);

// Validates user command fields
ParserUtil.verifyNoUnknownPrefix(args, AddCommand.MESSAGE_USAGE, "add",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_NOTE, PREFIX_RATING);
ParserUtil.verifyNoMissingField(argMultimap, AddCommand.MESSAGE_USAGE, "add",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_ADDRESS, PREFIX_PHONE, PREFIX_EMAIL);

if (!argMultimap.getPreamble().isEmpty()) {
boolean isPreambleEmpty = argMultimap.isPreambleEmpty();
if (!isPreambleEmpty) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddCommand.MESSAGE_USAGE));
}

argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS);

Person person = createPersonContact(argMultimap);

return new AddCommand(person);
}

/**
* Creates a person contact based on the argument multimap.
* @param argMultimap Contains the mappings of values to the specific prefixes.
* @return A person contact.
* @throws ParseException Thrown when invalid paramters are used.

Choose a reason for hiding this comment

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

repeated word "Thrown", follow same format as the other JavaDocs

i.e. @throws ParseException if invalid parameters are used.

*/
private Person createPersonContact(ArgumentMultimap argMultimap) throws ParseException {
try {
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).orElseThrow());
Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).orElseThrow());
Expand All @@ -70,9 +84,7 @@ public AddCommand parse(String args) throws ParseException {
Tag tag = new Tag("other");
Set<Tag> tags = new HashSet<>();
tags.add(tag);
Person person = new Person(name, phone, email, address, note, tags, rating);

return new AddCommand(person);
return new Person(name, phone, email, address, note, tags, rating);
} catch (ParseException pe) {
throw new ParseException(String.format(AddMessages.MESSAGE_ADD_INVALID_PARAMETERS, pe.getMessage()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,46 @@ public class AddMaintainerCommandParser implements Parser<AddMaintainerCommand>
private final Logger logger = LogsCenter.getLogger(getClass());
/**
* Parses the given {@code String} of arguments in the context of the AddStaffCommand
* and returns an AddCommand object for execution.
* and returns an AddCommand object for execution. Parameter args cannot be null.

Choose a reason for hiding this comment

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

same comment here on args

* @throws ParseException if the user input does not conform the expected format

Choose a reason for hiding this comment

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

same comment here on missing preposition "to"

*/
public AddMaintainerCommand parse(String args) throws ParseException {
assert (args != null) : "`argument` to pass for add maintainer command is null";

logger.log(Level.INFO, "Going to start parsing for add maintainer command.");
ParserUtil.verifyNoUnknownPrefix(args, AddMaintainerCommand.MESSAGE_USAGE, "add-maintainer",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_SKILL, PREFIX_COMMISSION, PREFIX_RATING);

ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_SKILL, PREFIX_COMMISSION, PREFIX_RATING);

// Validates user command fields
ParserUtil.verifyNoUnknownPrefix(args, AddMaintainerCommand.MESSAGE_USAGE, "add-maintainer",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_SKILL, PREFIX_COMMISSION, PREFIX_RATING);
ParserUtil.verifyNoMissingField(argMultimap, AddMaintainerCommand.MESSAGE_USAGE, "add-maintainer",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS, PREFIX_SKILL, PREFIX_COMMISSION);

if (!argMultimap.getPreamble().isEmpty()) {
boolean isPreambleEmpty = argMultimap.isPreambleEmpty();
if (!isPreambleEmpty) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddMaintainerCommand.MESSAGE_USAGE));
}

argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_SKILL, PREFIX_COMMISSION);

Maintainer person = createMaintainerContact(argMultimap);

return new AddMaintainerCommand(person);
}

/**
* Creates a maintainer contact based on the argument multimap.
* @param argMultimap Contains the mappings of values to the specific prefixes.
* @return A maintainer contact.
* @throws ParseException Thrown when invalid paramters are used.

Choose a reason for hiding this comment

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

same comment here on "Thrown"

*/
private Maintainer createMaintainerContact(ArgumentMultimap argMultimap) throws ParseException {
try {
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).orElseThrow());
Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).orElseThrow());
Expand All @@ -72,16 +87,12 @@ public AddMaintainerCommand parse(String args) throws ParseException {
String noteContent = argMultimap.getValue(PREFIX_NOTE).orElse("No note here");
Note note = noteContent.equals("No note here") ? new Note(noteContent) : ParserUtil.parseNote(noteContent);
Rating rating = ParserUtil.parseRating(argMultimap.getValue(PREFIX_RATING).orElse("0"));

Tag tag = new Tag("maintainer");
Set<Tag> tags = new HashSet<>();
tags.add(tag);
Skill skill = ParserUtil.parseSkill(argMultimap.getValue(PREFIX_SKILL).orElseThrow());
Commission commission = ParserUtil.parseCommission(argMultimap.getValue(PREFIX_COMMISSION).orElseThrow());

Maintainer person = new Maintainer(name, phone, email, address, note, tags, skill, commission, rating);

return new AddMaintainerCommand(person);
return new Maintainer(name, phone, email, address, note, tags, skill, commission, rating);
} catch (ParseException pe) {
throw new ParseException(String.format(AddMessages.MESSAGE_ADD_INVALID_PARAMETERS, pe.getMessage()));
}
Expand Down
34 changes: 23 additions & 11 deletions src/main/java/seedu/address/logic/parser/AddStaffCommandParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -39,31 +39,46 @@ public class AddStaffCommandParser implements Parser<AddStaffCommand> {

/**
* Parses the given {@code String} of arguments in the context of the AddStaffCommand
* and returns an AddCommand object for execution.
* and returns an AddCommand object for execution. Parameter args cannot be null.

Choose a reason for hiding this comment

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

same comment here on args

* @throws ParseException if the user input does not conform the expected format
*/
public AddStaffCommand parse(String args) throws ParseException {
assert (args != null) : "`argument` to pass for add staff command is null";

logger.log(Level.INFO, "Going to start parsing for add staff command.");
ParserUtil.verifyNoUnknownPrefix(args, AddStaffCommand.MESSAGE_USAGE, "add-staff",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_SALARY, PREFIX_EMPLOYMENT, PREFIX_RATING);

ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_SALARY, PREFIX_EMPLOYMENT, PREFIX_RATING);

// Validates user command fields
ParserUtil.verifyNoUnknownPrefix(args, AddStaffCommand.MESSAGE_USAGE, "add-staff",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_SALARY, PREFIX_EMPLOYMENT, PREFIX_RATING);
ParserUtil.verifyNoMissingField(argMultimap, AddStaffCommand.MESSAGE_USAGE, "add-staff",
FAILED_TO_ADD,
PREFIX_NAME, PREFIX_ADDRESS, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_EMPLOYMENT, PREFIX_SALARY);

if (!argMultimap.getPreamble().isEmpty()) {
boolean isPreambleEmpty = argMultimap.isPreambleEmpty();
if (!isPreambleEmpty) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddStaffCommand.MESSAGE_USAGE));
}

argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_SALARY, PREFIX_EMPLOYMENT);

Staff person = createStaffContact(argMultimap);

return new AddStaffCommand(person);
}

/**
* Creates a staff contact based on the argument multimap.
* @param argMultimap Contains the mappings of values to the specific prefixes.
* @return A staff contact.
* @throws ParseException Thrown when invalid paramters are used.

Choose a reason for hiding this comment

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

same comment here on "Thrown"

*/
private Staff createStaffContact(ArgumentMultimap argMultimap) throws ParseException {
try {
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).orElseThrow());
Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).orElseThrow());
Expand All @@ -77,10 +92,7 @@ public AddStaffCommand parse(String args) throws ParseException {
tags.add(tag);
Employment employment = ParserUtil.parseEmployment(argMultimap.getValue(PREFIX_EMPLOYMENT).orElseThrow());
Salary salary = ParserUtil.parseSalary(argMultimap.getValue(PREFIX_SALARY).orElseThrow());

Staff person = new Staff(name, phone, email, address, note, tags, salary, employment, rating);

return new AddStaffCommand(person);
return new Staff(name, phone, email, address, note, tags, salary, employment, rating);
} catch (ParseException pe) {
throw new ParseException(String.format(AddMessages.MESSAGE_ADD_INVALID_PARAMETERS, pe.getMessage()));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,29 +38,44 @@ public class AddSupplierCommandParser implements Parser<AddSupplierCommand> {
private final Logger logger = LogsCenter.getLogger(getClass());
/**
* Parses the given {@code String} of arguments in the context of the AddStaffCommand
* and returns an AddCommand object for execution.
* and returns an AddCommand object for execution. Parameter args cannot be null.

Choose a reason for hiding this comment

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

same comment here on args

* @throws ParseException if the user input does not conform the expected format
*/
public AddSupplierCommand parse(String args) throws ParseException {
assert (args != null) : "`argument` to pass for add supplier command is null";

logger.log(Level.INFO, "Going to start parsing for supplier command.");
ParserUtil.verifyNoUnknownPrefix(args, AddSupplierCommand.MESSAGE_USAGE, "add-supplier",
FAILED_TO_ADD, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_PRODUCT, PREFIX_PRICE, PREFIX_RATING);

ArgumentMultimap argMultimap =
ArgumentTokenizer.tokenize(args, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_PRODUCT, PREFIX_PRICE, PREFIX_RATING);

// Validates user command fields
ParserUtil.verifyNoUnknownPrefix(args, AddSupplierCommand.MESSAGE_USAGE, "add-supplier",
FAILED_TO_ADD, PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_PRODUCT, PREFIX_PRICE, PREFIX_RATING);
ParserUtil.verifyNoMissingField(argMultimap, AddSupplierCommand.MESSAGE_USAGE, "add-supplier",
FAILED_TO_ADD, PREFIX_NAME, PREFIX_ADDRESS, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_PRODUCT, PREFIX_PRICE);

if (!argMultimap.getPreamble().isEmpty()) {
boolean isPreambleEmpty = argMultimap.isPreambleEmpty();
if (!isPreambleEmpty) {
throw new ParseException(String.format(MESSAGE_INVALID_COMMAND_FORMAT, AddSupplierCommand.MESSAGE_USAGE));
}

argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS,
PREFIX_PRODUCT, PREFIX_PRICE);

Supplier person = createSupplierContact(argMultimap);

return new AddSupplierCommand(person);
}

/**
* Creates a supplier contact based on the argument multimap.
* @param argMultimap Contains the mappings of values to the specific prefixes.
* @return A supplier contact.
* @throws ParseException Thrown when invalid paramters are used.

Choose a reason for hiding this comment

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

same comment here on args

*/
private Supplier createSupplierContact(ArgumentMultimap argMultimap) throws ParseException {
try {
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).orElseThrow());
Phone phone = ParserUtil.parsePhone(argMultimap.getValue(PREFIX_PHONE).orElseThrow());
Expand All @@ -74,10 +89,7 @@ public AddSupplierCommand parse(String args) throws ParseException {
tags.add(tag);
Price price = ParserUtil.parsePrice(argMultimap.getValue(PREFIX_PRICE).orElseThrow());
Product product = ParserUtil.parseProduct(argMultimap.getValue(PREFIX_PRODUCT).orElseThrow());

Supplier person = new Supplier(name, phone, email, address, note, tags, product, price, rating);

return new AddSupplierCommand(person);
return new Supplier(name, phone, email, address, note, tags, product, price, rating);
} catch (ParseException pe) {
throw new ParseException(String.format(AddMessages.MESSAGE_ADD_INVALID_PARAMETERS, pe.getMessage()));
}
Expand Down
20 changes: 11 additions & 9 deletions src/main/java/seedu/address/logic/parser/DeleteCommandParser.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,30 +23,32 @@ 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.
* and returns a DeleteCommand object for execution. Parameter args cannot be null.

Choose a reason for hiding this comment

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

same here

* @throws ParseException if the user input does not conform the expected format

Choose a reason for hiding this comment

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

same comment here on missing preposition "to"

*/
public DeleteCommand parse(String args) throws ParseException {
assert (args != null) : "`argument` to pass for delete command is null";
logger.log(Level.INFO, "Going to start parsing for delete command.");

ParserUtil.verifyNoUnknownPrefix(args, DeleteCommand.MESSAGE_USAGE, "delete",
FAILED_TO_DELETE, PREFIX_NAME);
logger.log(Level.INFO, "Going to start parsing for delete command.");

Name name;

Choose a reason for hiding this comment

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

Why was the declaration of variable name shifted outside of the try-catch block? name is not being used anywhere outside of the try-catch block so shouldn't the declaration of the variable be inside of the block?

Following the course specification, "Variables should be initialized where they are declared and they should be declared in the smallest scope possible."

ArgumentMultimap argMultimap = ArgumentTokenizer.tokenize(args, PREFIX_NAME);

// Validates user command fields
ParserUtil.verifyNoUnknownPrefix(args, DeleteCommand.MESSAGE_USAGE, "delete",
FAILED_TO_DELETE, PREFIX_NAME);
ParserUtil.verifyNoMissingField(argMultimap, DeleteCommand.MESSAGE_USAGE, "delete",
FAILED_TO_DELETE, PREFIX_NAME);

if (!argMultimap.getPreamble().isEmpty()) {
argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS);
boolean isPreambleEmpty = argMultimap.getPreamble().isEmpty();
if (!isPreambleEmpty) {
logger.log(Level.WARNING, "Parsing error while parsing for delete command.");
throw new ParseException(String.format(DeleteMessages.MESSAGE_DELETE_MISSING_NAME,
DeleteCommand.MESSAGE_USAGE));
}

argMultimap.verifyNoDuplicatePrefixesFor(PREFIX_NAME, PREFIX_PHONE, PREFIX_EMAIL, PREFIX_ADDRESS);

try {
Name name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).orElseThrow());
name = ParserUtil.parseName(argMultimap.getValue(PREFIX_NAME).orElseThrow());
return new DeleteCommand(name);
} catch (ParseException pe) {
throw new ParseException(String.format(DeleteMessages.MESSAGE_DELETE_INVALID_PARAMETERS, pe.getMessage()));
Expand Down
Loading
Loading