From b3d0a71dec4c1f3c3b00f815a05003fb5e8c33f6 Mon Sep 17 00:00:00 2001 From: ljacqu Date: Sat, 21 Nov 2015 11:57:04 +0100 Subject: [PATCH] Merge ListUtil into StringUtil; refactor HelpSyntaxHelper + create test The HelpSyntaxHelper had suppressed warnings for string concatenation within StringBuilder - the point of the StringBuilder is that it is faster when you use it to concatenate many elements. If you still use string concatenation with + within these calls it beats the purpose. --- .../fr/xephi/authme/command/CommandParts.java | 15 +++-- .../authme/command/help/HelpSyntaxHelper.java | 63 +++++++++++-------- .../java/fr/xephi/authme/util/ListUtils.java | 58 ----------------- .../fr/xephi/authme/util/StringUtils.java | 35 +++++++++++ .../authme/command/CommandPartsTest.java | 38 +++++++++++ .../fr/xephi/authme/util/StringUtilsTest.java | 41 ++++++++++++ 6 files changed, 159 insertions(+), 91 deletions(-) delete mode 100644 src/main/java/fr/xephi/authme/util/ListUtils.java create mode 100644 src/test/java/fr/xephi/authme/command/CommandPartsTest.java diff --git a/src/main/java/fr/xephi/authme/command/CommandParts.java b/src/main/java/fr/xephi/authme/command/CommandParts.java index 2c67f7b1b..9e50cc164 100644 --- a/src/main/java/fr/xephi/authme/command/CommandParts.java +++ b/src/main/java/fr/xephi/authme/command/CommandParts.java @@ -3,7 +3,6 @@ package fr.xephi.authme.command; import java.util.ArrayList; import java.util.List; -import fr.xephi.authme.util.ListUtils; import fr.xephi.authme.util.StringUtils; /** @@ -165,8 +164,8 @@ public class CommandParts { * * @param other The other reference. * - - * @return The result from zero to above. A negative number will be returned on error. */ + * @return The result from zero to above. A negative number will be returned on error. + */ public double getDifference(CommandParts other) { return getDifference(other, false); } @@ -177,8 +176,8 @@ public class CommandParts { * @param other The other reference. * @param fullCompare True to compare the full references as far as the range reaches. * - - * @return The result from zero to above. A negative number will be returned on error. */ + * @return The result from zero to above. A negative number will be returned on error. + */ public double getDifference(CommandParts other, boolean fullCompare) { // Make sure the other reference is correct if(other == null) @@ -196,10 +195,10 @@ public class CommandParts { /** * Convert the parts to a string. * - - * @return The part as a string. */ + * @return The part as a string. + */ @Override public String toString() { - return ListUtils.implode(this.parts, " "); + return StringUtils.join(" ", this.parts); } } diff --git a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java index 5f564749b..0ff99a5cf 100644 --- a/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java +++ b/src/main/java/fr/xephi/authme/command/help/HelpSyntaxHelper.java @@ -1,13 +1,15 @@ package fr.xephi.authme.command.help; +import fr.xephi.authme.util.StringUtils; import org.bukkit.ChatColor; import fr.xephi.authme.command.CommandArgumentDescription; import fr.xephi.authme.command.CommandDescription; import fr.xephi.authme.command.CommandParts; -import fr.xephi.authme.util.ListUtils; /** + * Helper class for formatting a command's structure (name and arguments) + * for a Minecraft user. */ public final class HelpSyntaxHelper { @@ -16,52 +18,63 @@ public final class HelpSyntaxHelper { } /** - * Get the proper syntax for a command. + * Get the formatted syntax for a command. * - * @param commandDescription The command to get the syntax for. + * @param commandDescription The command to build the syntax for. * @param commandReference The reference of the command. * @param alternativeLabel The alternative label to use for this command syntax. * @param highlight True to highlight the important parts of this command. * * @return The command with proper syntax. */ - @SuppressWarnings("StringConcatenationInsideStringBufferAppend") - public static String getCommandSyntax(CommandDescription commandDescription, CommandParts commandReference, String alternativeLabel, boolean highlight) { - // Create a string builder to build the command - StringBuilder sb = new StringBuilder(); - - // Set the color and prefix a slash - sb.append(ChatColor.WHITE + "/"); + public static String getCommandSyntax(CommandDescription commandDescription, CommandParts commandReference, + String alternativeLabel, boolean highlight) { + // Create a string builder with white color and prefixed slash + StringBuilder sb = new StringBuilder() + .append(ChatColor.WHITE) + .append("/"); // Get the help command reference, and the command label CommandParts helpCommandReference = commandDescription.getCommandReference(commandReference); - final String parentCommand = (new CommandParts(helpCommandReference.getRange(0, helpCommandReference.getCount() - 1))).toString(); - String commandLabel = helpCommandReference.get(helpCommandReference.getCount() - 1); + final String parentCommand = new CommandParts( + helpCommandReference.getRange(0, helpCommandReference.getCount() - 1)).toString(); // Check whether the alternative label should be used - if (alternativeLabel != null && alternativeLabel.trim().length() > 0) { + String commandLabel; + if (StringUtils.isEmpty(alternativeLabel)) { + commandLabel = helpCommandReference.get(helpCommandReference.getCount() - 1); + } else { commandLabel = alternativeLabel; } // Show the important bit of the command, highlight this part if required - sb.append(ListUtils.implode(parentCommand, (highlight ? ChatColor.YELLOW + "" + ChatColor.BOLD : "") + commandLabel, " ")); - if(highlight) - sb.append(ChatColor.YELLOW); + sb.append(parentCommand) + .append(" ") + .append(highlight ? ChatColor.YELLOW.toString() + ChatColor.BOLD : "") + .append(commandLabel); - // Add each command arguments - for(CommandArgumentDescription arg : commandDescription.getArguments()) { - // Add the argument as optional or non-optional argument - if(!arg.isOptional()) - sb.append(ChatColor.ITALIC + " <" + arg.getLabel() + ">"); - else - sb.append(ChatColor.ITALIC + " [" + arg.getLabel() + "]"); + if (highlight) { + sb.append(ChatColor.YELLOW); + } + + // Add each command argument + for (CommandArgumentDescription arg : commandDescription.getArguments()) { + sb.append(ChatColor.ITALIC).append(formatArgument(arg)); } // Add some dots if the command allows unlimited arguments - if(commandDescription.getMaximumArguments() < 0) - sb.append(ChatColor.ITALIC + " ..."); + if (commandDescription.getMaximumArguments() < 0) { + sb.append(ChatColor.ITALIC).append(" ..."); + } // Return the build command syntax return sb.toString(); } + + private static String formatArgument(CommandArgumentDescription argument) { + if (argument.isOptional()) { + return " [" + argument.getLabel() + "]"; + } + return " <" + argument.getLabel() + ">"; + } } diff --git a/src/main/java/fr/xephi/authme/util/ListUtils.java b/src/main/java/fr/xephi/authme/util/ListUtils.java deleted file mode 100644 index 69ed092ef..000000000 --- a/src/main/java/fr/xephi/authme/util/ListUtils.java +++ /dev/null @@ -1,58 +0,0 @@ -package fr.xephi.authme.util; - -import java.util.ArrayList; -import java.util.List; - -/** - */ -public class ListUtils { - - /** - * Implode a list of elements into a single string, with a specified separator. - * - * @param elements The elements to implode. - * @param separator The separator to use. - * - - * @return The result string. */ - public static String implode(List elements, String separator) { - // Create a string builder - StringBuilder sb = new StringBuilder(); - - // Append each element - for(String element : elements) { - // Make sure the element isn't empty - if(element.trim().length() == 0) - continue; - - // Prefix the separator if it isn't the first element - if(sb.length() > 0) - sb.append(separator); - - // Append the element - sb.append(element); - } - - // Return the result - return sb.toString(); - } - - /** - * Implode two elements into a single string, with a specified separator. - * - * @param element The first element to implode. - * @param otherElement The second element to implode. - * @param separator The separator to use. - * - - * @return The result string. */ - public static String implode(String element, String otherElement, String separator) { - // Combine the lists - List combined = new ArrayList<>(); - combined.add(element); - combined.add(otherElement); - - // Implode and return the result - return implode(combined, separator); - } -} diff --git a/src/main/java/fr/xephi/authme/util/StringUtils.java b/src/main/java/fr/xephi/authme/util/StringUtils.java index e9b495bcd..f0dd3d9ce 100644 --- a/src/main/java/fr/xephi/authme/util/StringUtils.java +++ b/src/main/java/fr/xephi/authme/util/StringUtils.java @@ -49,4 +49,39 @@ public class StringUtils { return false; } + /** + * Null-safe method for checking whether a string is empty. Note that the string + * is trimmed, so this method also considers a string with whitespace as empty. + * + * @param str the string to verify + * + * @return true if the string is empty, false otherwise + */ + public static boolean isEmpty(String str) { + return str == null || str.trim().isEmpty(); + } + + /** + * Joins a list of elements into a single string with the specified delimiter. + * + * @param delimiter the delimiter to use + * @param elements the elements to join + * + * @return a new String that is composed of the elements separated by the delimiter + */ + public static String join(String delimiter, Iterable elements) { + StringBuilder sb = new StringBuilder(); + for (String element : elements) { + if (!isEmpty(element)) { + // Add the separator if it isn't the first element + if (sb.length() > 0) { + sb.append(delimiter); + } + sb.append(element); + } + } + + return sb.toString(); + } + } diff --git a/src/test/java/fr/xephi/authme/command/CommandPartsTest.java b/src/test/java/fr/xephi/authme/command/CommandPartsTest.java new file mode 100644 index 000000000..62d22b5fa --- /dev/null +++ b/src/test/java/fr/xephi/authme/command/CommandPartsTest.java @@ -0,0 +1,38 @@ +package fr.xephi.authme.command; + +import org.junit.Test; + +import java.util.Arrays; + +import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertThat; + +/** + * Test for {@link CommandParts}. + */ +public class CommandPartsTest { + + @Test + public void shouldPrintPartsForStringRepresentation() { + // given + CommandParts parts = new CommandParts(Arrays.asList("some", "parts", "for", "test")); + + // when + String str = parts.toString(); + + // then + assertThat(str, equalTo("some parts for test")); + } + + @Test + public void shouldPrintEmptyStringForNoArguments() { + // given + CommandParts parts = new CommandParts(); + + // when + String str = parts.toString(); + + // then + assertThat(str, equalTo("")); + } +} diff --git a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java index 668f2059b..51cd96776 100644 --- a/src/test/java/fr/xephi/authme/util/StringUtilsTest.java +++ b/src/test/java/fr/xephi/authme/util/StringUtilsTest.java @@ -2,8 +2,13 @@ package fr.xephi.authme.util; import org.junit.Test; +import java.util.Arrays; +import java.util.List; + import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; /** * Test for {@link StringUtils}. @@ -43,4 +48,40 @@ public class StringUtilsTest { // then assertThat(result, equalTo(false)); } + + @Test + public void shouldCheckIsEmptyUtil() { + // Should be true for null/empty/whitespace + assertTrue(StringUtils.isEmpty(null)); + assertTrue(StringUtils.isEmpty("")); + assertTrue(StringUtils.isEmpty(" \t")); + + // Should be false if string has content + assertFalse(StringUtils.isEmpty("P")); + assertFalse(StringUtils.isEmpty(" test")); + } + + @Test + public void shouldJoinString() { + // given + List elements = Arrays.asList("test", "for", null, "join", "StringUtils"); + + // when + String result = StringUtils.join(", ", elements); + + // then + assertThat(result, equalTo("test, for, join, StringUtils")); + } + + @Test + public void shouldNotHaveDelimiter() { + // given + List elements = Arrays.asList(" ", null, "\t", "hello", null); + + // when + String result = StringUtils.join("-", elements); + + // then + assertThat(result, equalTo("hello")); + } }