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.

Sign up for GitHub

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

Jump to bottom

Review for A0143157J's collated code #183

Closed
mauris opened this issue Apr 3, 2017 · 2 comments
Closed

Review for A0143157J's collated code #183

mauris opened this issue Apr 3, 2017 · 2 comments

Comments

@mauris
Copy link

mauris commented Apr 3, 2017 •
edited

  • InvalidDatesException has an unexpected/unused empty line.
  • AddCommand constructor could have a bit more SLAP-ed
  • FindCommandParser.parseFindDates seems to have lots of if-statements and different cases, which increases code complexity and kind of is a sign for smelly code
  • ModelManager.renameCategory seems to be handling a lot of things
  • Recurrence.updateTaskDates definitely no-go.
  • UniqueTaskList.sort is also one long method
  • tests well implemented
@tanwei0319
Copy link

Hi Sam, can I ask what's wrong with Recurrence.updateTaskDates? Is it because of the markTaskUndone(task)?

@mauris
Copy link
Author

mauris commented Apr 3, 2017

each case can be SLAP-ed further (:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@mauris @tanwei0319