Remove unnecessary Guava usages #13
Conversation
|
|
There was a problem hiding this comment.
Thank you very much for this!
When I started writing Brigadier in 2015, we didn't have this luxury. I think perhaps we could even remove Guava entirely at this point. I'm on an airplane right now and can't check myself - were there any remaining usages of Guava after this change?
|
I couldn't spot any other guava usages in the source, but there are plenty in the tests. https://github.com/Mojang/brigadier/search?p=1&q=common.collect&unscoped_q=common.collect |
|
Then perhaps we can make it only required for testing? |
|
@MrMicky-FR change 'api' -> 'testCompile' here |
|
I removed some others Guava usages, just one to remove now: https://github.com/Mojang/brigadier/blob/master/src/main/java/com/mojang/brigadier/tree/CommandNode.java#L179-L183 |
|
I finished removing Guava 😃 Just I have'nt tested it yet |
| return getSortedKey().compareTo(o.getSortedKey()); | ||
| } | ||
|
|
||
| return (o instanceof LiteralCommandNode) ? 1 : -1; |
There was a problem hiding this comment.
I think this should return 0 if they're both, or neither, instanceof. I am not sure that will have any practical effects in the current codebase but it could skew any pre-sorting.
There was a problem hiding this comment.
This code returns the same as the actual code with Guava ComparisonChain but I can change it if you prefer.
There was a problem hiding this comment.
I don't think it does. The ComparisonChain should return 0 if neither are literals (therefore preserving any existing sort), but this will return -1.
There was a problem hiding this comment.
Oh thanks, I corrected this
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| public class CommandContext<S> { | ||
|
|
||
| public static final Map<Class<?>, Class<?>> PRIMITIVE_TO_WRAPPER = new HashMap<>(); |
There was a problem hiding this comment.
Perhaps make this private so it's not an official part of the Brigadier API.
Remove unnecessary Guava usages like
Lists.newArrayList()that can be replace with justnew ArrayList<>()https://google.github.io/guava/releases/23.0/api/docs/com/google/common/collect/Lists.html#newArrayList--
Note for Java 7 and later: this method is now unnecessary and should be treated as deprecated. Instead, use the ArrayList constructor directly, taking advantage of the new "diamond" syntax.