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

Minor refactorings and code style changes #807

Merged
merged 5 commits into from
Oct 23, 2018
Merged

Conversation

npathai
Copy link
Contributor

@npathai npathai commented Oct 20, 2018

  • Removed several use of raw types
  • Removed unnecessary throws clauses
  • Used lambda expressions wherever applicable
  • Used apt assertion methods for readability
  • Use of try with resources wherever applicable
  • Corrected incorrect order of assertXXX arguments

…re not needed, changed incorrect order of arguments in assertEquals
…raw types 2) Removed unnecessary throws clauses 3) Used lambda expressions wherever applicable 4) Used apt assertion methods for readability 5) Use of try with resources wherever applicable 6) Corrected incorrect order of assertXXX arguments
@@ -58,12 +59,12 @@ public void testFirstDataMapper() {
mapper.update(student);

/* Check if student is updated in db */
assertEquals(mapper.find(student.getStudentId()).get().getName(), "AdamUpdated");
assertEquals("AdamUpdated", mapper.find(student.getStudentId()).get().getName());

Choose a reason for hiding this comment

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

we should use student.getName() instead of AdamUpdated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair point. I din't use student.getName() exactly but rather used a local variable at both places.

@iluwatar
Copy link
Owner

Looks good to me

@npathai
Copy link
Contributor Author

npathai commented Oct 21, 2018

@iluwatar Done with review changes.

Copy link

@IAmPramod IAmPramod left a comment

Choose a reason for hiding this comment

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

Looks good. LGTM.

@npathai
Copy link
Contributor Author

npathai commented Oct 23, 2018

@iluwatar Merging this PR as I got two 👍 s

@npathai npathai merged commit 2aa9e78 into master Oct 23, 2018
@iluwatar
Copy link
Owner

iluwatar commented Nov 4, 2018

Thanks @npathai, well done 👍

@iluwatar
Copy link
Owner

@all-contributors please add @npathai for code ideas maintenance mentoring question review

@allcontributors
Copy link
Contributor

@iluwatar

This project's configuration file has malformed JSON: .all-contributorsrc. Error:: Unexpected string in JSON at position 484

@iluwatar
Copy link
Owner

@all-contributors please add @npathai for code ideas maintenance mentoring question review

@allcontributors
Copy link
Contributor

@iluwatar

I've put up a pull request to add @npathai! 🎉

@iluwatar
Copy link
Owner

#798

@iluwatar iluwatar deleted the MinorRefactorings branch December 30, 2022 15:16
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.

3 participants