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

Reduced code smell #3090

Closed
wants to merge 4 commits into from
Closed

Reduced code smell #3090

wants to merge 4 commits into from

Conversation

matteobaccan
Copy link

I have clean up some code smell

  • Modifiers should be declared in the correct order (S1124)
  • Unused local variable (S1481)
  • Use of diamond <> operator (S2293)
  • Use collection.isEmpty() (S1155)

I hope this may be useful for this project

Copy link
Contributor

@katzyn katzyn left a comment

Choose a reason for hiding this comment

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

Thanks, but I don't see any reason in rearrangement of modifiers, such changes only create a noise in the history. New order isn't better than old one.

Code in h2/src/tools with exception for build package also doesn't need any changes, because it's just a playground. This code isn't used anywhere.

@@ -881,7 +881,7 @@ static String getDataTypeRegistrationKey(DataType<?> dataType) {
registerDataType(getKeyType());
}
if (getValueType() == null) {
setValueType((DataType<? super V>) new VersionedValueType<V,Object>(defaultDataType));
setValueType((DataType<? super V>) new VersionedValueType<>(defaultDataType));
Copy link
Contributor

Choose a reason for hiding this comment

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

After this change cast also needs to be removed to avoid compiler warning.

Copy link
Author

Choose a reason for hiding this comment

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

Sorry but, I have rebuilt H2 with maven and I have not seen the warning.

@matteobaccan
Copy link
Author

Thanks, but I don't see any reason in rearrangement of modifiers, such changes only create a noise in the history. New order isn't better than old one.

Code in h2/src/tools with exception for build package also doesn't need any changes, because it's just a playground. This code isn't used anywhere.

Hi

all modifications are code cleanup, not optimizations.

I have run some static code analyzer that suggests some modification in order to reduce code smell and use standard code practice.

These modifications don't improve the program, only allow to have a better code quality.

For this reason, you can, obviously, reject all the push or keep only part of the push (for example the Unused local variable (S1481)).

all the best
matteo baccan

andreitokar added a commit to andreitokar/h2database that referenced this pull request Nov 25, 2021
@andreitokar
Copy link
Contributor

I took some non-controversial peaces to master as part of #3210

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.

None yet

3 participants