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

Beining/fix done command bugs #256

Merged
merged 5 commits into from
Nov 5, 2020
Merged

Conversation

dearvae
Copy link

@dearvae dearvae commented Nov 4, 2020

fix #236 fix #220

@dearvae dearvae added this to the v1.4 milestone Nov 4, 2020
@dearvae dearvae self-assigned this Nov 4, 2020
@dearvae dearvae changed the title fix done command bugs Beining/fix done command bugs Nov 5, 2020
}
return new DoneCommand(indexes, durations);
} catch (ParseException pe) {
throw new ParseException(
String.format(MESSAGE_INVALID_COMMAND_FORMAT, pe.getMessage(), DoneCommand.MESSAGE_USAGE), pe);
} catch (NumberFormatException ne) {
throw new ParseException("should be a positive number and the maximum duration is 2147483647 minutes");
Copy link

Choose a reason for hiding this comment

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

Magic number

Copy link

Choose a reason for hiding this comment

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

MAX_INT = .....
then string format

@codecov-io
Copy link

codecov-io commented Nov 5, 2020

Codecov Report

Merging #256 into master will increase coverage by 0.39%.
The diff coverage is 46.15%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #256      +/-   ##
============================================
+ Coverage     43.06%   43.46%   +0.39%     
- Complexity      570      579       +9     
============================================
  Files           114      114              
  Lines          2835     2860      +25     
  Branches        421      435      +14     
============================================
+ Hits           1221     1243      +22     
  Misses         1481     1481              
- Partials        133      136       +3     
Impacted Files Coverage Δ Complexity Δ
.../seedu/address/logic/parser/DoneCommandParser.java 47.36% <0.00%> (-12.64%) 3.00 <0.00> (ø)
.../main/java/seedu/address/ui/card/DeadlineCard.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ava/seedu/address/storage/JsonAdaptedDeadline.java 69.01% <60.00%> (+0.89%) 13.00 <0.00> (+1.00)
...va/seedu/address/model/task/deadline/Duration.java 76.92% <100.00%> (ø) 5.00 <2.00> (-1.00)
.../java/seedu/address/commons/util/DateTimeUtil.java 97.14% <0.00%> (+0.36%) 8.00% <0.00%> (+3.00%)
...eedu/address/logic/parser/LessonCommandParser.java 74.50% <0.00%> (+13.39%) 14.00% <0.00%> (+6.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4edbe60...3aba91a. Read the comment docs.

@@ -19,9 +19,10 @@ public void isValidTitle() {

Copy link

Choose a reason for hiding this comment

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

Why is this called isValidTitle??? Rename

@@ -19,9 +19,10 @@ public void isValidTitle() {

// invalid duration
assertFalse(Duration.isValidDuration(-10)); // negative number
Copy link

Choose a reason for hiding this comment

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

Test a few more values in each equivalence partition and especially near boundary values. Please include values such as 0, -1, some valid values and values near MAX_INT

Copy link

@BobbyZhouZijian BobbyZhouZijian left a comment

Choose a reason for hiding this comment

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

LGTM

@BobbyZhouZijian BobbyZhouZijian merged commit 2814a6d into master Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants