Lots of Sonar and Java 6 plus Pretty Readme #270
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
hazendaz
commented
Sep 22, 2014
hazendaz
commented
- Overrides added across library.
- Mostly minor sonar issue work.
- Updated clirr version of code to compare.
- Change two object creations using internal classes to just create objects inline reducing complexity of how that worked.
- Readme is now markdown
- The following table shows before and after sonar results with this PR
| Current Issues Before PR | After PR |
|---|---|
| Blocker 25 | Blocker 25 |
| Critical 4 | Critical 4 |
| Major 600 | Major 572 |
| Minor 159 | Minor 104 |
| Info 25 | Info 25 |
Adding remaining override annotations across mybatis.
Update the CLIRR comparison version to 3.2.7 which was last released and ordered in properties section alphabetically.
Removed some spacing that was on empty lines or trailing end of lines.
Try not to allow use of auto boxing as it is unclear.
Use smart 'if' statement on returns where check was for null and returning null otherwise value. This does fix one return per method in some cases per sonar.
Array list was being returned using non standard way of creating the array as essentially an internal class. No real good reason for this and it was being flagged via sonar for not exposing private constructor to prevent synthetic access. Much cleaner approach is to just build the array without the internal class.
Fix possible null pointer exception where developer was trying to outsmart the null pointer checks by using a flag for null pointer. This caused eclipse to see a potential null pointer even though there was not one. Since the check was only used once, just inlined it.
Removed some exceptions that were not actually thrown.
When 'return' occurs in 'if' statement, there is no reason to have an else statement. While this is in quite a few places, just cleaned up a couple of them. One for clarity and the other to move a line ending comment that sonar complained about.
Add public constructors so synthetic access is not used per sonar. Also fix accessors.
Private constructors needed in static final classes to prevent instantiation of the class. Fixed some types on maps & various other objects.
Unchecked BindingException, don't throw it, that is redundant.
Two overrides turned out to be jdk7 only. Comment them out and add note regarding this. Also fixed trailing comment.
Move object to correct location in class.
Prevent instantiation using private constructor in static classes.
Prevent instantiation of static class. Avoid autoboxing and unnecessary object creation.
Override that only calls super method is not necessary as it inherits super class.
Do not create objects and immediately return them. Just return directly.
Change NodeHandler to not be an internal class creating itself but rather a method in default scope. Have it perform the get and simply return the NodeHandler as that is the only usage.
Changed to markdown. Added badges for travis CI and maven central. Added mybatis logo.
Move comments to line before trailing usage.
Just return objects directly rather than creating another one.
Move private constructor to expected location.
Prevent instantiation of static class. Remove trailing comment that served no purpose. Please note that if the intent was to not print the remainder of the next line, it was not actually doing that and was printing it. So this just removes the ending line comment. It does appear the entire thing needed printed.
|
Wow. Thats a great work! (And too long for me to review so there it goes! ;) ) Thank you! |
emacarron
added a commit
that referenced
this pull request
Sep 22, 2014
Lots of Sonar and Java 6 plus Pretty Readme
|
Thank you! --- Original Message --- From: "Eduardo Macarron" [email protected] Wow. Thats a great work! (And too long for me to review so there it goes! ;) ) Thank you! Reply to this email directly or view it on GitHub: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.