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

Machine-applicable refactorings #1224

Merged
merged 2 commits into from
Jun 11, 2022
Merged

Machine-applicable refactorings #1224

merged 2 commits into from
Jun 11, 2022

Conversation

petervdonovan
Copy link
Collaborator

@petervdonovan petervdonovan commented Jun 11, 2022

I did a quick (< 2 hour) pass through some files related to the C generator because the warnings appearing in the IDE were distracting. The only changes that should make any semantic difference to our code are replacement of == with .equals to compare strings.

In the interest of minimizing conflicts (and of avoiding controversy), I tried to do this quickly and neglected many more potential issues that IntelliJ flagged. Maybe some more "code health" passes of this sort should happen if we think it's a good idea.

Changes include:

  • Replacing redundant type arguments with diamond types
  • Removing redundant toString() calls in string concatenation
  • Using pattern matching to avoid casting (after using instanceof)
  • Fixing broken references in Javadoc comments
  • Deleting or updating outdated method documentation
  • Replacing usages of == and != to compare strings! This seems like a real bug, and it surprises me that it was not caught in CI.

Changes do not include:

  • Removing unnecessary parentheses
  • Always surrounding + with spaces when used for string concatenation
  • Making members final
  • Deleting unused class variables
  • Deleting unused methods
  • Deleting unused parameters
  • Deleting useless local variables
  • Addressing warnings that point to logical errors in the control flow of methods
  • Addressing "wrong tag" warnings for comments of the form "@author{Name Surname [email protected]}" that make "@author{Name" look like all one JavaDoc tag
  • Refactoring "pseudo functional style code" that causes side effects, even though it uses APIs designed for functional programming

@petervdonovan petervdonovan changed the title Machine-applicable changes. Machine-applicable refactorings. Jun 11, 2022
@@ -1081,19 +1081,19 @@ private void generateReactorChildren(
private void pickCompilePlatform() {
var OS = System.getProperty("os.name").toLowerCase();
Copy link
Member

Choose a reason for hiding this comment

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

The uppercase is a bit weird here. I'd write osn or something like that.

Copy link
Member

@lhstrh lhstrh left a comment

Choose a reason for hiding this comment

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

Thanks for these cleanups!

@lhstrh lhstrh added the refactoring Code quality enhancement label Jun 11, 2022
@lhstrh lhstrh changed the title Machine-applicable refactorings. Machine-applicable refactorings Jun 11, 2022
@petervdonovan petervdonovan merged commit a013ba8 into master Jun 11, 2022
@petervdonovan petervdonovan deleted the nits branch June 11, 2022 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Code quality enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants