-
-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Conversation
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()); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
acyclic-visitor/src/test/java/com/iluwatar/acyclicvisitor/ConfigureForUnixVisitorTest.java
Outdated
Show resolved
Hide resolved
monostate/src/main/java/com/iluwatar/monostate/LoadBalancer.java
Outdated
Show resolved
Hide resolved
Looks good to me |
@iluwatar Done with review changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. LGTM.
@iluwatar Merging this PR as I got two 👍 s |
Thanks @npathai, well done 👍 |
@all-contributors please add @npathai for code ideas maintenance mentoring question review |
This project's configuration file has malformed JSON: .all-contributorsrc. Error:: Unexpected string in JSON at position 484 |
@all-contributors please add @npathai for code ideas maintenance mentoring question review |
I've put up a pull request to add @npathai! 🎉 |