Skip to content

Fixed code smells after running sonarqube on the project #1429

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

Merged
merged 4 commits into from
Aug 25, 2020
Merged

Fixed code smells after running sonarqube on the project #1429

merged 4 commits into from
Aug 25, 2020

Conversation

abhijaykumar
Copy link
Contributor

Multiple code smells were found by SonarQube on the project. Some of them were violations of the below rules:

  • Local variable names in java must be with the default regular expression ^[a-z][a-zA-Z0-9]*$ RSPEC-117
  • The diamond operator ("<>") should be used when the type of the maps are already defined RSPEC-2293
  • Package names being camel cased is a code smell, all package names must be lowercase
  • String literals should not be duplicated RSPEC-1192
  • Generic types shouldn't be used raw (without type parameters) in variable declarations or return values. Doing so bypasses generic type checking, and defers the catch of unsafe code to runtime. RSPEC-3740
  • The dangling else problem appears when nested if/else statements are written without curly braces. In this case, else is associated with the nearest if but that is not always obvious and sometimes the indentation can also be misleading.RSPEC-5261

Here is the overall SonarQube report for the repo (Development) branch post the fixes.

image

This pull request brings down the code smells from 188 to 177.

- Local variable names in java must be with the  default regular expression ^[a-z][a-zA-Z0-9]*$
https://rules.sonarsource.com/java/tag/convention/RSPEC-117
- The diamond operator ("<>") should be used when the type of the maps are already defined
https://rules.sonarsource.com/java/RSPEC-2293
- Package names being camel cased is a code smell
-Generic types shouldn't be used raw (without type parameters) in variable declarations or return values. Doing so bypasses generic type checking, and defers the catch of unsafe code to runtime. https://rules.sonarsource.com/java/RSPEC-3740
- The dangling else problem appears when nested if/else statements are written without curly braces. In this case, else is associated with the nearest if but that is not always obvious and sometimes the indentation can also be misleading.
https://rules.sonarsource.com/java/tag/confusing/RSPEC-5261
@abhijaykumar abhijaykumar marked this pull request as draft August 21, 2020 15:36
@abhijaykumar abhijaykumar marked this pull request as ready for review August 21, 2020 15:36
@abhijaykumar
Copy link
Contributor Author

@yanglbme Please review this.
Thanks

@drabbit75
Copy link
Member

have the changed codes been compiled and tested? if so I think they can be merged.

@abhijaykumar
Copy link
Contributor Author

@drabbit75 Yes, all tests are passing after the changes. I was able to generate the code coverage report too.

Copy link
Member

@realDuYuanChao realDuYuanChao left a comment

Choose a reason for hiding this comment

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

LGTM

@realDuYuanChao
Copy link
Member

@abhijaykumar cloud you reopen a new PR and fix build error? Thanks

@abhijaykumar
Copy link
Contributor Author

abhijaykumar commented Aug 25, 2020

@abhijaykumar cloud you reopen a new PR and fix build error? Thanks

@shellhub Where's the build error? I ran gradlew clean test and it builds fine.

@realDuYuanChao
Copy link
Member

@abhijaykumar
Copy link
Contributor Author

@shellhub Thanks. Looks like it passed for me since I'm on JDK11. I'll rebuild on JDK8 and create a new PR.

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 this pull request may close these issues.

3 participants