patternjavaMinor
Functional interface uses uncheck or unsafe operations
Viewed 0 times
operationsusesunsafeinterfacefunctionaluncheck
Problem
First of all, I am absolutely surprised that this code even works. Originally what I did was create a version checking system for Minecraft plugins (specifically Bukkit, Craftbukkit, & Spigot).
What I wanted to do was refactor my
So instead of the constructors accepting Bukkit classes, I changed it to accept a generic String that would represent the version. Then I made Builder methods, which I deleted and move into a factory:
So, the main methods of Version.java are
However, any such checks on specific Bukkit classes and objects would restrict the use of my Version checking system to only Bukkit. And I wanted to make it platform independent.
So what I did was create a functional interface called
What is surprising is that I can pass type
Version.java
```
package mc.euro.version;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
* The Version object: Capable of asking the important questions:.
*
* Is the version that's currently installed on the server comp
What I wanted to do was refactor my
Version class in order to make it platform independent (where it doesn't depend or provide functionality for only Minecraft servers and plugins).So instead of the constructors accepting Bukkit classes, I changed it to accept a generic String that would represent the version. Then I made Builder methods, which I deleted and move into a factory:
VersionFactory (this makes it easier for end-users to construct Version objects specific to Bukkit servers).So, the main methods of Version.java are
isCompatible() and isSupported(). However, the first thing these methods did was perform an isEnabled() check on the Version object because any compatibility checks should fail (as intended) if the target plugin isn't even installed on the server.However, any such checks on specific Bukkit classes and objects would restrict the use of my Version checking system to only Bukkit. And I wanted to make it platform independent.
So what I did was create a functional interface called
Tester, and asked for it in the constructor of Version (along with the object under test). The primary intent is to allow plugin.isEnabled() to be run and return true or false BEFORE isCompatible() or isSupported() is executed. However, it does allow ANY custom test() to be performed.What is surprising is that I can pass type
Object into the constructor, which gets assigned to a field, and ultimately gets passed to tester.isEnabled() which is able to call a plugin specific method on a generic Object type!Version.java
```
package mc.euro.version;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
/**
* The Version object: Capable of asking the important questions:.
*
* Is the version that's currently installed on the server comp
Solution
if (x >= 0) {
return true;
}
return false;
if (x <= 0) {
return true;
}
return false;
if (matcher.find()) {
return true;
}
return false;Are constructs I'd express as
return x >= 0;and
return x <= 0;and
return matcher.find();instead.
@Override
public int compareTo(Version whichVersion) {
int[] currentVersion = parseVersion(this.version);
int[] otherVersion = parseVersion(whichVersion.toString());
int length = (currentVersion.length >= otherVersion.length) ? currentVersion.length : otherVersion.length;
for (int index = 0; index otherVersion.length) {
return currentVersion[index] - 0;
} else if (currentVersion.length < otherVersion.length) {
return 0 - otherVersion[index];
}
}
}
return 0;
}index = index + 1 can be index++.for
return currentVersion[index] - 0;, it's silly to subtract 0 as it does nothing.index = otherVersion.length) ? currentVersion.length : otherVersion.length;, consider using Math.max(currentVersion.length, otherVersion.length) instead.The reason I would express all these statements differently is because most of those things can simply be expressed in less code. The statement
index <= (length - 1) is not easier to understand, nor does it provide any semantic value over index < length. By removing these overly complex syntactical structures, you make it easier to understand the code.Code Snippets
if (x >= 0) {
return true;
}
return false;
if (x <= 0) {
return true;
}
return false;
if (matcher.find()) {
return true;
}
return false;return x >= 0;return x <= 0;return matcher.find();@Override
public int compareTo(Version whichVersion) {
int[] currentVersion = parseVersion(this.version);
int[] otherVersion = parseVersion(whichVersion.toString());
int length = (currentVersion.length >= otherVersion.length) ? currentVersion.length : otherVersion.length;
for (int index = 0; index <= (length - 1); index = index + 1) {
try {
if (currentVersion[index] != otherVersion[index]) {
return currentVersion[index] - otherVersion[index];
}
} catch (IndexOutOfBoundsException ex) {
if (currentVersion.length > otherVersion.length) {
return currentVersion[index] - 0;
} else if (currentVersion.length < otherVersion.length) {
return 0 - otherVersion[index];
}
}
}
return 0;
}Context
StackExchange Code Review Q#64607, answer score: 5
Revisions (0)
No revisions yet.