-
Notifications
You must be signed in to change notification settings - Fork 5
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
Improve code quality #304
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
* @throws ParseException if the user input does not conform the expected format | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. repeated word "Thrown", follow same format as the other JavaDocs i.e. |
||
*/ | ||
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()); | ||
|
@@ -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())); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment here on |
||
* @throws ParseException if the user input does not conform the expected format | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()); | ||
|
@@ -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())); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment here on |
||
* @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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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()); | ||
|
@@ -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())); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment here on |
||
* @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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same comment here on |
||
*/ | ||
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()); | ||
|
@@ -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())); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was the declaration of variable 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())); | ||
|
There was a problem hiding this comment.
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'