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

Edit task command #100

Merged
merged 4 commits into from Oct 20, 2019
Merged

Edit task command #100

merged 4 commits into from Oct 20, 2019

Conversation

junnbang
Copy link

@junnbang junnbang commented Oct 19, 2019

Added Edit task command
-refer to commits for updates

*Storage for tasks not done
-will do by Sunday.

@junnbang junnbang added this to the v1.3 milestone Oct 19, 2019
@junnbang junnbang self-assigned this Oct 19, 2019
@junnbang junnbang requested a review from a team October 19, 2019 17:52
String startTimeStr = durationArr[0];
String endTimeStr = durationArr[2];

return parse(startTimeStr, endTimeStr);
Copy link

Choose a reason for hiding this comment

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

Since this is a public facing static method, you may want to apply the regex check to duration to ensure that the input is in the correct form? Otherwise, duration[i] may throw IndexOutOfBoundError.

Copy link
Author

Choose a reason for hiding this comment

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

Okay noted, because usually we check if is valid then we parse, so i didn't include it.

But for safety, i will include isValidDuration under the parse and create a IncorrectFormat exception for it to throw

Copy link
Author

Choose a reason for hiding this comment

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

adding assert instead

* Example: 1200 - 1330.
*/
public static String getStringFromDuration(EventTime duration) {
DateTimeFormatter jsonFormatter = DateTimeFormatter.ofPattern("Hmm");
Copy link

Choose a reason for hiding this comment

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

Maybe extract to a static field

Copy link
Author

Choose a reason for hiding this comment

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

okay noted

@junnbang junnbang modified the milestones: v1.3, v1.2 Oct 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants