Standardize error message for inputs with valid command format but invalid index #161
Conversation
Codecov Report
@@ Coverage Diff @@
## master #161 +/- ##
============================================
- Coverage 73.61% 73.25% -0.36%
- Complexity 637 639 +2
============================================
Files 100 100
Lines 1944 1959 +15
Branches 225 226 +1
============================================
+ Hits 1431 1435 +4
- Misses 424 434 +10
- Partials 89 90 +1
Continue to review full report at Codecov.
|
|
|
||
| /** | ||
| * Parses {@code oneBasedIndex} into an {@code Index} and returns it. Leading and trailing whitespaces will be | ||
| * trimmed. | ||
| * @throws ParseException if the specified index is invalid (not non-zero unsigned integer). | ||
| */ | ||
| public static Index parseIndex(String oneBasedIndex) throws ParseException { | ||
| public static Index parseIndex(String oneBasedIndex) throws CommandException, ParseException { |
There was a problem hiding this comment.
We should only throw ParseException in this function.
CommandException is used for errors which occur during execurtion of a command.
ParserException Represents a parse error encountered by a parser.
In this case, the error occurs during the parsing stage when the users passes in -1.
| if (!StringUtil.isNonZeroUnsignedInteger(trimmedIndex)) { | ||
| if (StringUtil.isInteger(trimmedIndex) && !StringUtil.isNonZeroUnsignedInteger(trimmedIndex)) { | ||
| throw new CommandException(Messages.MESSAGE_INVALID_PERSON_DISPLAYED_INDEX); | ||
| } else if (!StringUtil.isNonZeroUnsignedInteger(trimmedIndex)) { | ||
| throw new ParseException(MESSAGE_INVALID_INDEX); | ||
| } | ||
| return Index.fromOneBased(Integer.parseInt(trimmedIndex)); |
There was a problem hiding this comment.
There is a redundancy here,
we called Integer.parseInt twice,
once in StringUtil.isInteger and once in this line.
This is because the parser is structured in a "validate first, then parse".
Would suggest a parse dont validate approach instead: https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/
There was a problem hiding this comment.
This is more of a FYI, up to you if you want to pull the change in :)
rnmy
force-pushed
the
update-edit-command
branch
from
2b5f99f
to
6b38038
Compare
rnmy
force-pushed
the
update-edit-command
branch
from
6b38038
to
8ad37a8
Compare
closes #113, #122