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

Feedback for the collated code of A0148095X #178

Closed
cmkumar87 opened this issue Oct 30, 2016 · 0 comments · Fixed by #187
Closed

Feedback for the collated code of A0148095X #178

cmkumar87 opened this issue Oct 30, 2016 · 0 comments · Fixed by #187
Assignees

Comments

@cmkumar87
Copy link

  • Put literals in a variable.
    return "Todo list data load completed. Task list size: "
  • consider more conventional names. Perhaps also shorter, if possible.

    notifySaveLocationChange instead of private void indicateChangeSaveLocationRequest(String location) {
  • Thats a mouthful! public void handleChangeSaveLocationRequestEvent(ChangeSaveLocationRequestEvent event) {

    Are you doing this to avoid a header comment! It has other side effects.
  • Tests not written properly. Follow convention. One method per test case. We want to know what exactly failed when a build fails. Name test case according to the convention taught in the earlier tutorial.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants