Add gather command by tags #109
Conversation
Codecov Report
@@ Coverage Diff @@
## master #109 +/- ##
============================================
+ Coverage 76.29% 76.53% +0.23%
- Complexity 544 571 +27
============================================
Files 88 91 +3
Lines 1772 1828 +56
Branches 178 186 +8
============================================
+ Hits 1352 1399 +47
- Misses 372 375 +3
- Partials 48 54 +6
... and 7 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
I think it's quite a good idea to use a Command Pattern to implement the gather command. Nice work!
As a side note, perhaps it would be more readable to rename GatherEmails to a noun instead of a verb like GatherEmailPrompt or something since it would make it easier to reason about, as well as being more intuitive for first-timers.
Also, putting these into a separate folder would make the file structure more neat.
| public static final String FINANCIAL_PLAN_REGEX = "^[a-zA-Z0-9\\s]+"; | ||
| public static final String TAG_REGEX = "\\p{Alnum}+"; |
There was a problem hiding this comment.
Maybe can abstract validation regex to the respective field classes
| public static final String FINANCIAL_PLAN_CONSTRAINTS = "PROMPT should be alphanumeric or space characters"; | ||
| public static final String TAG_CONSTRAINTS = "Tags names should be alphanumeric"; |
There was a problem hiding this comment.
Maybe can abstract messages to respective field classes too
| if (trimmedArgs.contains(PREFIX_FINANCIAL_PLAN.getPrefix())) { | ||
| // Parse Financial Plan | ||
| argDescription = trimmedArgs.replace(PREFIX_FINANCIAL_PLAN.getPrefix(), "").trim(); | ||
| validateInput(argDescription, FINANCIAL_PLAN_REGEX, FINANCIAL_PLAN_CONSTRAINTS); | ||
| return new GatherCommand(new GatherEmailsByFinancialPlan(argDescription)); | ||
| } else if (trimmedArgs.contains(PREFIX_TAG.getPrefix())) { | ||
| // Parse Tag | ||
| argDescription = trimmedArgs.replace(PREFIX_TAG.getPrefix(), "").trim(); | ||
| validateInput(argDescription, TAG_REGEX, TAG_CONSTRAINTS); | ||
| return new GatherCommand(new GatherEmailsByTag(argDescription)); | ||
| } | ||
|
|
||
| if (!trimmedArgs.matches(VALIDATION_REGEX)) { | ||
| throw new ParseException( | ||
| String.format(MESSAGE_INVALID_COMMAND_FORMAT, GatherCommand.MESSAGE_USAGE)); | ||
| } |
There was a problem hiding this comment.
Could consider using ArgumentMultiMap class instead, can try refering to the EditCommandParser. This implementation might introduce bugs, for example, what happens if the prefix given by the person is ot/ and the contains method checks that PREFIX_TAG is present?
Add gather command by tags for enhancement