Skip to content

Commit

Permalink
Code clean up from Sonar Cloud analysis (#2068)
Browse files Browse the repository at this point in the history
* Code clean up from Sonar Cloud analysis

* Fix tests

* Remove code smell

* Rename "island" which hides the field declared at line 25.

* Removed code smells.

* Rename variable record to rec

Renamed "record" variable to not match a restricted identifier.
Restricted Identifiers should not be used as identifiers. "record" is
using in Java 16.

* Added private constructor to prevent instantiation of static class

Changed variable name to rec instead of restricted "record".

* Remove Blueprint code smells.

* Use a record for database settings constructor

Code smell: Methods should not have too many parameters. I'm not sure
what methods are using this class though.

* Update MyWorlds version

The POM for MyWorlds is invalid and causes a warning, but this still
persists with this version.

* Extracted nested try block into a separate method.

Makes it clear when reading the code what might be caught

* Extracted nested try block into a separate method.

* Fixed JavaDoc /** instead of just /*

* Extracted nested try block into a separate method.

* Refactored to not assign loop counter from within the loop body.

* Better delete option. With results.

That said, this is legacy code to handle an issue that occurred a long
time ago and this whole set of code can probably be removed.

* Catch Exceptions not Throwable

* Log error with BentoBox logError

* Use computeIfAbsent

Using these instead leads to cleaner and more readable code.

* User can no longer be null

* Added the missing @deprecated annotation and @SInCE ref

* Added @SInCE reference

* Merge if statements

* Use BentoBox error logging.

* Added JavaDoc @SInCE

* Remove deprecated class and move used class

* Remove deprecated WoodType and use Type.

* Remove unused import

* Extracted nested try block into a separate method.

* Comment empty default statement

* Clean up logic; avoid switch

* Use Java instead of Guava

* private constructor to hide the implicit public one.

* Private constructor to hide the implicit public one.

Merged if statement.

* Add comment

* if merge

* Make variable constant

* Remove unused imports

* Remove deprecated and unused method

* Remove unused import

* Typo

* Remove instanceof and cast

* Remove superfluous null check

* Put constant at bottom of file because @BONNe likes it there.

* Simplify particle validation code
  • Loading branch information
tastybento authored Jan 1, 2023
1 parent 0183380 commit 056cff4
Show file tree
Hide file tree
Showing 49 changed files with 596 additions and 654 deletions.
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@
<placeholderapi.version>2.10.9</placeholderapi.version>
<githubapi.version>d5f5e0bbd8</githubapi.version>
<dynmap.version>3.0-SNAPSHOT</dynmap.version>
<myworlds.version>1.19-v2</myworlds.version>
<myworlds.version>1.19.3-v1</myworlds.version>
<!-- Revision variable removes warning about dynamic version -->
<revision>${build.version}-SNAPSHOT</revision>
<!-- Do not change unless you want different name for local builds. -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,9 @@ public boolean canExecute(User user, String label, List<String> args) {
}

private boolean getIsland(User user, List<String> args) {
if (args.get(0).equalsIgnoreCase(SPAWN_ISLAND)) {
if (getIslands().getSpawn(getWorld()).isPresent()) {
island = getIslands().getSpawn(getWorld()).get();
return true;
}
if (args.get(0).equalsIgnoreCase(SPAWN_ISLAND) && getIslands().getSpawn(getWorld()).isPresent()) {
island = getIslands().getSpawn(getWorld()).get();
return true;
}
// Get target player
@Nullable UUID targetUUID = Util.getUUID(args.get(0));
Expand Down Expand Up @@ -191,17 +189,18 @@ public boolean execute(User user, String label, List<String> args) {
// Command line setting
flag.ifPresent(f -> {
switch (f.getType()) {
case PROTECTION -> {
island.setFlag(f, rank);
getIslands().save(island);
}
case SETTING -> {
island.setSettingsFlag(f, activeState);
getIslands().save(island);
}
case WORLD_SETTING -> f.setSetting(getWorld(), activeState);
default -> {
}
case PROTECTION -> {
island.setFlag(f, rank);
getIslands().save(island);
}
case SETTING -> {
island.setSettingsFlag(f, activeState);
getIslands().save(island);
}
case WORLD_SETTING -> f.setSetting(getWorld(), activeState);
default -> {
// Do nothing
}
}
});
user.sendMessage("general.success");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public boolean canExecute(User user, String label, List<String> args)
return false;
}

if (clipboard.getBlueprint().getBedrock() == null)
if (clipboard.getBlueprint() != null && clipboard.getBlueprint().getBedrock() == null)
{
// Bedrock is required for all blueprints.
user.sendMessage("commands.admin.blueprint.bedrock-required");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@ private void delete(Island island, User user, String name) {
@Override
public Optional<List<String>> tabComplete(User user, String alias, List<String> args) {
String lastArg = !args.isEmpty() ? args.get(args.size()-1) : "";
Island island = getIslands().getIsland(getWorld(), user.getUniqueId());
if (island != null) {
return Optional.of(Util.tabLimit(new ArrayList<>(island.getHomes().keySet()), lastArg));
Island is = getIslands().getIsland(getWorld(), user.getUniqueId());
if (is != null) {
return Optional.of(Util.tabLimit(new ArrayList<>(is.getHomes().keySet()), lastArg));
} else {
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,11 @@ public boolean canExecute(User user, String label, List<String> args) {
user.sendMessage(Flags.PREVENT_TELEPORT_WHEN_FALLING.getHintReference());
return false;
}
if (!args.isEmpty()) {
if (!getIslands().isHomeLocation(island, String.join(" ", args))) {
user.sendMessage("commands.island.go.unknown-home");
user.sendMessage("commands.island.sethome.homes-are");
island.getHomes().keySet().stream().filter(s -> !s.isEmpty()).forEach(s -> user.sendMessage("commands.island.sethome.home-list-syntax", TextVariables.NAME, s));
return false;
}
if (!args.isEmpty() && !getIslands().isHomeLocation(island, String.join(" ", args))) {
user.sendMessage("commands.island.go.unknown-home");
user.sendMessage("commands.island.sethome.homes-are");
island.getHomes().keySet().stream().filter(s -> !s.isEmpty()).forEach(s -> user.sendMessage("commands.island.sethome.home-list-syntax", TextVariables.NAME, s));
return false;
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,9 +87,9 @@ public boolean execute(User user, String label, List<String> args) {
@Override
public Optional<List<String>> tabComplete(User user, String alias, List<String> args) {
String lastArg = !args.isEmpty() ? args.get(args.size()-1) : "";
Island island = getIslands().getIsland(getWorld(), user.getUniqueId());
if (island != null) {
return Optional.of(Util.tabLimit(new ArrayList<>(island.getHomes().keySet()), lastArg));
Island is = getIslands().getIsland(getWorld(), user.getUniqueId());
if (is != null) {
return Optional.of(Util.tabLimit(new ArrayList<>(is.getHomes().keySet()), lastArg));
} else {
return Optional.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,22 +96,24 @@ private void acceptTrustInvite(User user, Invite invite) {
// Remove the invite
itc.removeInvite(playerUUID);
User inviter = User.getInstance(invite.getInviter());
if (inviter != null) {
Island island = getIslands().getIsland(getWorld(), inviter);
if (island != null) {
if (island.getMemberSet(RanksManager.TRUSTED_RANK, false).size() > getIslands().getMaxMembers(island, RanksManager.TRUSTED_RANK)) {
user.sendMessage("commands.island.team.trust.is-full");
return;
}
island.setRank(user, RanksManager.TRUSTED_RANK);
IslandEvent.builder()
.island(island)
.involvedPlayer(user.getUniqueId())
.admin(false)
.reason(IslandEvent.Reason.RANK_CHANGE)
.rankChange(island.getRank(user), RanksManager.TRUSTED_RANK)
.build();
Island island = getIslands().getIsland(getWorld(), inviter);
if (island != null) {
if (island.getMemberSet(RanksManager.TRUSTED_RANK, false).size() > getIslands().getMaxMembers(island, RanksManager.TRUSTED_RANK)) {
user.sendMessage("commands.island.team.trust.is-full");
return;
}
island.setRank(user, RanksManager.TRUSTED_RANK);
IslandEvent.builder()
.island(island)
.involvedPlayer(user.getUniqueId())
.admin(false)
.reason(IslandEvent.Reason.RANK_CHANGE)
.rankChange(island.getRank(user), RanksManager.TRUSTED_RANK)
.build();
if (inviter.isOnline()) {
inviter.sendMessage("commands.island.team.trust.success", TextVariables.NAME, user.getName());
}
if (inviter.isPlayer()) {
user.sendMessage("commands.island.team.trust.you-are-trusted", TextVariables.NAME, inviter.getName());
}
}
Expand All @@ -121,22 +123,24 @@ private void acceptCoopInvite(User user, Invite invite) {
// Remove the invite
itc.removeInvite(playerUUID);
User inviter = User.getInstance(invite.getInviter());
if (inviter != null) {
Island island = getIslands().getIsland(getWorld(), inviter);
if (island != null) {
if (island.getMemberSet(RanksManager.COOP_RANK, false).size() > getIslands().getMaxMembers(island, RanksManager.COOP_RANK)) {
user.sendMessage("commands.island.team.coop.is-full");
return;
}
island.setRank(user, RanksManager.COOP_RANK);
IslandEvent.builder()
.island(island)
.involvedPlayer(user.getUniqueId())
.admin(false)
.reason(IslandEvent.Reason.RANK_CHANGE)
.rankChange(island.getRank(user), RanksManager.COOP_RANK)
.build();
Island island = getIslands().getIsland(getWorld(), inviter);
if (island != null) {
if (island.getMemberSet(RanksManager.COOP_RANK, false).size() > getIslands().getMaxMembers(island, RanksManager.COOP_RANK)) {
user.sendMessage("commands.island.team.coop.is-full");
return;
}
island.setRank(user, RanksManager.COOP_RANK);
IslandEvent.builder()
.island(island)
.involvedPlayer(user.getUniqueId())
.admin(false)
.reason(IslandEvent.Reason.RANK_CHANGE)
.rankChange(island.getRank(user), RanksManager.COOP_RANK)
.build();
if (inviter.isOnline()) {
inviter.sendMessage("commands.island.team.coop.success", TextVariables.NAME, user.getName());
}
if (inviter.isPlayer()) {
user.sendMessage("commands.island.team.coop.you-are-a-coop-member", TextVariables.NAME, inviter.getName());
}
}
Expand Down Expand Up @@ -179,7 +183,7 @@ private void acceptTeamInvite(User user, Invite invite) {
}
user.sendMessage("commands.island.team.invite.accept.you-joined-island", TextVariables.LABEL, getTopLabel());
User inviter = User.getInstance(invite.getInviter());
if (inviter != null) {
if (inviter.isOnline()) {
inviter.sendMessage("commands.island.team.invite.accept.name-joined-your-island", TextVariables.NAME, user.getName());
}
getIslands().save(teamIsland);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,11 +96,11 @@ private Map<Integer, PanelItem> populateInventoryPanel()
{
for (int k = 0; k < this.panelTemplate.content()[i].length; k++)
{
ItemTemplateRecord record = this.panelTemplate.content()[i][k];
ItemTemplateRecord rec = this.panelTemplate.content()[i][k];

if (record != null && record.dataMap().containsKey("type"))
if (rec != null && rec.dataMap().containsKey("type"))
{
String type = String.valueOf(record.dataMap().get("type"));
String type = String.valueOf(rec.dataMap().get("type"));

int counter = this.typeSlotMap.computeIfAbsent(type, key -> 0);
this.typeSlotMap.put(type, counter + 1);
Expand Down Expand Up @@ -226,11 +226,11 @@ private Map<Integer, PanelItem> populateHopperPanel()
// Analyze the template
for (int i = 0; i < 5; i++)
{
ItemTemplateRecord record = this.panelTemplate.content()[0][i];
ItemTemplateRecord rec = this.panelTemplate.content()[0][i];

if (record != null && record.dataMap().containsKey("type"))
if (rec != null && rec.dataMap().containsKey("type"))
{
String type = String.valueOf(record.dataMap().get("type"));
String type = String.valueOf(rec.dataMap().get("type"));

int counter = this.typeSlotMap.computeIfAbsent(type, key -> 0);
this.typeSlotMap.put(type, counter + 1);
Expand Down Expand Up @@ -289,11 +289,11 @@ private Map<Integer, PanelItem> populateDropperPanel()
{
for (int k = 0; k < 3; k++)
{
ItemTemplateRecord record = this.panelTemplate.content()[i][k];
ItemTemplateRecord rec = this.panelTemplate.content()[i][k];

if (record != null && record.dataMap().containsKey("type"))
if (rec != null && rec.dataMap().containsKey("type"))
{
String type = String.valueOf(record.dataMap().get("type"));
String type = String.valueOf(rec.dataMap().get("type"));

int counter = this.typeSlotMap.computeIfAbsent(type, key -> 0);
this.typeSlotMap.put(type, counter + 1);
Expand Down Expand Up @@ -354,41 +354,41 @@ private Map<Integer, PanelItem> populateDropperPanel()

/**
* This method passes button creation from given record template.
* @param record Template of the button that must be created.
* @param rec Template of the button that must be created.
* @return PanelItem of the template, otherwise null.
*/
@Nullable
private PanelItem makeButton(@Nullable ItemTemplateRecord record)
private PanelItem makeButton(@Nullable ItemTemplateRecord rec)
{
if (record == null)
if (rec == null)
{
// Immediate exit if record is null.
return null;
}

if (record.dataMap().containsKey("type"))
if (rec.dataMap().containsKey("type"))
{
// If dataMap is not null, and it is not empty, then pass button to the object creator function.

return this.makeAddonButton(record);
return this.makeAddonButton(rec);
}
else
{
PanelItemBuilder itemBuilder = new PanelItemBuilder();

if (record.icon() != null)
if (rec.icon() != null)
{
itemBuilder.icon(record.icon().clone());
itemBuilder.icon(rec.icon().clone());
}

if (record.title() != null)
if (rec.title() != null)
{
itemBuilder.name(this.user.getTranslation(record.title()));
itemBuilder.name(this.user.getTranslation(rec.title()));
}

if (record.description() != null)
if (rec.description() != null)
{
itemBuilder.description(this.user.getTranslation(record.description()));
itemBuilder.description(this.user.getTranslation(rec.description()));
}

// If there are generic click handlers that could be added, then this is a place
Expand All @@ -402,19 +402,19 @@ private PanelItem makeButton(@Nullable ItemTemplateRecord record)

/**
* This method passes button to the type creator, if that exists.
* @param record Template of the button that must be created.
* @param rec Template of the button that must be created.
* @return PanelItem of the button created by typeCreator, otherwise null.
*/
@Nullable
private PanelItem makeAddonButton(@NonNull ItemTemplateRecord record)
private PanelItem makeAddonButton(@NonNull ItemTemplateRecord rec)
{
// Get object type.
String type = String.valueOf(record.dataMap().getOrDefault("type", ""));
String type = String.valueOf(rec.dataMap().getOrDefault("type", ""));

if (!this.typeCreators.containsKey(type))
{
// There are no object with a given type.
return this.makeFallBack(record.fallback());
return this.makeFallBack(rec.fallback());
}

BiFunction<ItemTemplateRecord, ItemSlot, PanelItem> buttonBuilder = this.typeCreators.get(type);
Expand All @@ -426,48 +426,48 @@ private PanelItem makeAddonButton(@NonNull ItemTemplateRecord record)
this.typeIndex.put(type, itemSlot.nextItemSlot());

// Try to get next object.
PanelItem item = buttonBuilder.apply(record, itemSlot);
return item == null ? this.makeFallBack(record.fallback()) : item;
PanelItem item = buttonBuilder.apply(rec, itemSlot);
return item == null ? this.makeFallBack(rec.fallback()) : item;
}


/**
* This method creates a fall back button for given record.
* @param record Record which fallback must be created.
* @param rec Record which fallback must be created.
* @return PanelItem if fallback was creates successfully, otherwise null.
*/
@Nullable
private PanelItem makeFallBack(@Nullable ItemTemplateRecord record)
private PanelItem makeFallBack(@Nullable ItemTemplateRecord rec)
{
return record == null ? null : this.makeButton(record.fallback());
return rec == null ? null : this.makeButton(rec.fallback());
}


/**
* This method translates template record into a panel item.
* @param record Record that must be translated.
* @param rec Record that must be translated.
* @return PanelItem that contains all information from the record.
*/
private PanelItem makeTemplate(PanelTemplateRecord.TemplateItem record)
private PanelItem makeTemplate(PanelTemplateRecord.TemplateItem rec)
{
PanelItemBuilder itemBuilder = new PanelItemBuilder();

// Read icon only if it is not null.
if (record.icon() != null)
if (rec.icon() != null)
{
itemBuilder.icon(record.icon().clone());
itemBuilder.icon(rec.icon().clone());
}

// Read title only if it is not null.
if (record.title() != null)
if (rec.title() != null)
{
itemBuilder.name(this.user.getTranslation(record.title()));
itemBuilder.name(this.user.getTranslation(rec.title()));
}

// Read description only if it is not null.
if (record.description() != null)
if (rec.description() != null)
{
itemBuilder.description(this.user.getTranslation(record.description()));
itemBuilder.description(this.user.getTranslation(rec.description()));
}

// Click Handlers are managed by custom addon buttons.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable;


/**
* This Record contains all necessary information about Item Template that can be used to craft panel item.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import world.bentobox.bentobox.api.panels.Panel;


/**
* This is template object for the panel reader. It contains data that can exist in the panel.
* PanelBuilder will use this to build panel.
Expand Down
Loading

0 comments on commit 056cff4

Please sign in to comment.