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.

Sign up for GitHub

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

Jump to bottom

Code cleanup #637

Open
wants to merge 54 commits into
base: master
Choose a base branch
from
Open

Code cleanup #637

wants to merge 54 commits into from

Conversation

svaningelgem
Copy link
Contributor

Thanks a lot @booksaw for your fixes!

I added a (lot more) issues my IDE noticed. Each commit is 1 particular issue.
Mostly harmless.

Everything compiles fine 😀

@svaningelgem
Remove unused import.
a6882c1
@svaningelgem
field can be final
4ea1eaf
@svaningelgem
Use chained StringBuilder.append calls instead of String concatenat…
d0f9a44 
…ion.
@svaningelgem
Unnecessary toString() calls.
04cb9f7
@svaningelgem
Remove unnecessary modifiers (public isn't needed in interfaces, priv…
51400dd 
…ate isn't needed in enums)
@svaningelgem
Remove unnecessary ;
4560754
@svaningelgem
A few more isEmpty
8aab3b9
@svaningelgem
Simplify if-statement (it would be returning null anyway)
675718a
@svaningelgem
Redundant local variable
a789dae
@svaningelgem
Method call is always inverted - suppress warning
bafd027
@svaningelgem
declaration can have final modifier
5e0368d
@svaningelgem
Define as a constant
0b47b75
@svaningelgem
exception is never thrown
88fd326
@svaningelgem
Remove unused imports
95166e2
@svaningelgem
Method is identical to its super method.
734aabc
@svaningelgem
Unnecessary unicode escape sequence.
d043383
@svaningelgem
Use enhanced for-loop
305d066
@svaningelgem
Explicit type can be replaced by <>
c59061c
@svaningelgem
Cleanup lambda functions
88c35a0
@svaningelgem
Javadoc issues corrected
7904a32
@svaningelgem
Inner classes may be static
a7c9ff1
@svaningelgem
toArray just needs the type, not the actual array to fill.
cbf30c8
@svaningelgem
Chain append is more performant than concatenation
b89279c
@svaningelgem
str concatenation in a loop
f393aa7
@svaningelgem
Yet a few more isEmpty-s
67db59f
@svaningelgem
Not annotated method overrides method annotated with @NotNull
df2c1a9
@svaningelgem
found is always null. Make this clear
a19e697
@svaningelgem
Condition is always 'true'
e0a2115
@svaningelgem
Condition 'echest != null' is always 'true'
f28c7ba
@svaningelgem
'Stream<Path>' used without 'try'-with-resources statement
f58dfeb
@svaningelgem svaningelgem changed the title Optimize imports Code cleanup Jul 23, 2024
booksaw
booksaw requested changes Jul 30, 2024
View reviewed changes
Copy link
Owner

@booksaw booksaw left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for taking a look, most problems I have noticed so far are related to indentation.

Just as a word of warning, I am very hesitant to PRs that have these many minor changes in as in the past it has caused problems that took several months of bug reports to clean up where unintentionally behaviour has been changed. (Mainly because BetterTeams has no automated testing and with a diff this large, I can't reasonably manually test every change)

src/main/java/com/booksaw/betterTeams/commands/team/DisbandCommand.java Outdated Show resolved Hide resolved
src/main/java/com/booksaw/betterTeams/commands/teama/TeleportTeama.java Outdated Show resolved Hide resolved
src/main/java/com/booksaw/betterTeams/database/api/Database.java Outdated Show resolved Hide resolved
src/main/java/com/booksaw/betterTeams/database/api/Database.java Outdated Show resolved Hide resolved
src/main/java/com/booksaw/betterTeams/integrations/placeholder/TeamPlaceholderOptionsEnum.java Outdated Show resolved Hide resolved
@booksaw
Copy link
Owner

booksaw commented Jul 30, 2024

By the way, thanks for separating changes into different commits, it makes it a lot easier to review :)

@svaningelgem
Optimize key lookup & removal.
e2f9df0
@svaningelgem
Indentation
c8f44e3
@svaningelgem
Merge remote-tracking branch 'refs/remotes/origin/master' into optimi…
1517c3f 
…ze-imports

# Conflicts:
#	src/main/java/com/booksaw/betterTeams/team/storage/storageManager/SeparatedYamlStorageManager.java
@svaningelgem
Copy link
Contributor Author

Just as a word of warning, I am very hesitant to PRs that have these many minor changes.
I completely understand. Problem is once I started with the code validation in my IDE, I just kept adding stuff.
Luckily the commit-by-commit approach seems to improve that a bit for you.

Normally in this PR there are no real changes. Just cleanup of your excellent codebase based on IntelliJ's comments about it.

@svaningelgem
Merge remote-tracking branch 'refs/remotes/origin/master' into optimi…
1aabbcc 
…ze-imports

# Conflicts:
#	src/main/java/com/booksaw/betterTeams/database/api/Database.java
#	src/main/java/com/booksaw/betterTeams/events/MCTeamManagement.java
@svaningelgem svaningelgem requested a review from booksaw August 7, 2024 06:05
@svaningelgem
Merge remote-tracking branch 'refs/remotes/booksaw/master' into optim…
827c921 
…ize-imports

# Conflicts:
#	pom.xml
@svaningelgem
Adding some compiler flags to warn when using deprecation & unchecked…
2686a33 
… calls.

+ adding source encoding (on win I got a warning about resources being implicitly utf8, and that I should define it explicitly)
@svaningelgem
Copy link
Contributor Author

Hi @booksaw , any chance you got lately to continue your review of this PR?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@booksaw booksaw Awaiting requested review from booksaw

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@svaningelgem @booksaw