From 6acb5680cc685afc071ca5bcc358d31690bb1786 Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Thu, 13 Feb 2014 10:24:56 +0000
Subject: [PATCH] Code cleanups: - Factorized code - Fixed Sonar violations - Removed duplicated lines reported by Sonar - Applied refactorings suggested by AutoRefactor plugin 

---
 opendj-server/src/main/java/org/forgerock/opendj/server/setup/cli/SetupCli.java         |  130 ++++------
 opendj-cli/src/main/java/com/forgerock/opendj/cli/CommandBuilder.java                   |   36 +-
 opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/MakeLDIFTestCase.java |   18 -
 opendj-cli/src/main/java/com/forgerock/opendj/cli/CommonArguments.java                  |   76 +++---
 opendj-cli/src/main/java/com/forgerock/opendj/cli/MultiColumnPrinter.java               |    6 
 opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/Model.java          |   25 +-
 opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/RuntimeOptions.java |   35 +-
 opendj-server/src/test/java/org/forgerock/opendj/server/setup/cli/SetupCliTestCase.java |   27 -
 opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentParser.java                   |  315 ++++++++++---------------
 9 files changed, 277 insertions(+), 391 deletions(-)

diff --git a/opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentParser.java b/opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentParser.java
index ca53343..d32b495 100644
--- a/opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentParser.java
+++ b/opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentParser.java
@@ -95,72 +95,94 @@
      */
     private BooleanArgument noPropertiesFileArgument;
 
-    // The argument that will be used to trigger the display of usage
-    // information.
+    /**
+     * The argument that will be used to trigger the display of usage
+     * information.
+     */
     private Argument usageArgument;
 
-    // The argument that will be used to trigger the display of the OpenDJ
-    // version.
+    /**
+     * The argument that will be used to trigger the display of the OpenDJ
+     * version.
+     */
     private Argument versionArgument;
 
-    // The set of unnamed trailing arguments that were provided for this
-    // parser.
+    /**
+     * The set of unnamed trailing arguments that were provided for this parser.
+     */
     private final ArrayList<String> trailingArguments;
 
-    // Indicates whether this parser will allow additional unnamed
-    // arguments at the end of the list.
+    /**
+     * Indicates whether this parser will allow additional unnamed arguments at
+     * the end of the list.
+     */
     private final boolean allowsTrailingArguments;
 
-    // Indicates whether long arguments should be treated in a
-    // case-sensitive manner.
+    /**
+     * Indicates whether long arguments should be treated in a case-sensitive
+     * manner.
+     */
     private final boolean longArgumentsCaseSensitive;
 
-    // Indicates whether the usage or version information has been
-    // displayed.
+    /**
+     * Indicates whether the usage or version information has been displayed.
+     */
     private boolean usageOrVersionDisplayed;
 
-    // Indicates whether the version argument was provided.
+    /** Indicates whether the version argument was provided. */
     private boolean versionPresent;
 
-    // The set of arguments defined for this parser, referenced by short
-    // ID.
+    /**
+     * The set of arguments defined for this parser, referenced by short ID.
+     */
     private final HashMap<Character, Argument> shortIDMap;
 
-    // The set of arguments defined for this parser, referenced by
-    // argument name.
+    /**
+     * The set of arguments defined for this parser, referenced by argument
+     * name.
+     */
     private final HashMap<String, Argument> argumentMap;
 
-    // The set of arguments defined for this parser, referenced by long
-    // ID.
+    /**
+     * The set of arguments defined for this parser, referenced by long ID.
+     */
     private final HashMap<String, Argument> longIDMap;
 
-    // The maximum number of unnamed trailing arguments that may be
-    // provided.
+    /**
+     * The maximum number of unnamed trailing arguments that may be provided.
+     */
     private final int maxTrailingArguments;
 
-    // The minimum number of unnamed trailing arguments that may be
-    // provided.
+    /**
+     * The minimum number of unnamed trailing arguments that may be provided.
+     */
     private final int minTrailingArguments;
 
-    // The total set of arguments defined for this parser.
+    /** The total set of arguments defined for this parser. */
     private final LinkedList<Argument> argumentList;
 
-    // The output stream to which usage information should be printed.
+    /** The output stream to which usage information should be printed. */
     private OutputStream usageOutputStream;
 
-    // The fully-qualified name of the Java class that should be invoked
-    // to launch the program with which this argument parser is associated.
+    /**
+     * The fully-qualified name of the Java class that should be invoked to
+     * launch the program with which this argument parser is associated.
+     */
     private final String mainClassName;
 
-    // A human-readable description for the tool, which will be included
-    // when displaying usage information.
+    /**
+     * A human-readable description for the tool, which will be included when
+     * displaying usage information.
+     */
     private final LocalizableMessage toolDescription;
 
-    // The display name that will be used for the trailing arguments in
-    // the usage information.
+    /**
+     * The display name that will be used for the trailing arguments in the
+     * usage information.
+     */
     private final String trailingArgsDisplayName;
 
-    // The raw set of command-line arguments that were provided.
+    /** The raw set of command-line arguments that were provided. */
     private String[] rawArguments;
 
     /** Set of argument groups. */
@@ -195,9 +217,9 @@
     private final ArgumentGroup generalArgGroup = new ArgumentGroup(INFO_DESCRIPTION_GENERAL_ARGS
             .get(), Integer.MIN_VALUE);
 
-    private final static String INDENT = "    ";
+    private static final String INDENT = "    ";
 
-    private final static int MAX_LENGTH = 80;
+    private static final int MAX_LENGTH = 80;
 
     /**
      * Creates a new instance of this argument parser with no arguments. Unnamed
@@ -322,24 +344,21 @@
     public void addArgument(final Argument argument, ArgumentGroup group) throws ArgumentException {
 
         final Character shortID = argument.getShortIdentifier();
-        if ((shortID != null) && shortIDMap.containsKey(shortID)) {
+        if (shortID != null && shortIDMap.containsKey(shortID)) {
             final String conflictingName = shortIDMap.get(shortID).getName();
-
-            final LocalizableMessage message =
-                    ERR_ARGPARSER_DUPLICATE_SHORT_ID.get(argument.getName(), String
-                            .valueOf(shortID), conflictingName);
-            throw new ArgumentException(message);
+            throw new ArgumentException(
+                    ERR_ARGPARSER_DUPLICATE_SHORT_ID.get(argument.getName(), shortID, conflictingName));
         }
 
-        if (versionArgument != null) {
-            if (shortID != null && shortID.equals(versionArgument.getShortIdentifier())) {
-                // Update the version argument to not display its short identifier.
-                try {
-                    versionArgument = getVersionArgument(false);
-                    this.generalArgGroup.addArgument(versionArgument);
-                } catch (final ArgumentException e) {
-                    // ignore
-                }
+        if (versionArgument != null
+                && shortID != null
+                && shortID.equals(versionArgument.getShortIdentifier())) {
+            // Update the version argument to not display its short identifier.
+            try {
+                versionArgument = getVersionArgument(false);
+                this.generalArgGroup.addArgument(versionArgument);
+            } catch (final ArgumentException e) {
+                // ignore
             }
         }
 
@@ -382,7 +401,7 @@
 
     /**
      * Adds the provided argument to the set of arguments handled by this parser
-     * and puts the arguement in the default group.
+     * and puts the argument in the default group.
      *
      * @param argument
      *            The argument to be added.
@@ -396,7 +415,7 @@
 
     /**
      * Adds the provided argument to the set of arguments handled by this parser
-     * and puts the arguement in the general group.
+     * and puts the argument in the general group.
      *
      * @param argument
      *            The argument to be added.
@@ -461,7 +480,7 @@
      */
     Properties checkExternalProperties() throws ArgumentException {
         // We don't look for properties file.
-        if ((noPropertiesFileArgument != null) && (noPropertiesFileArgument.isPresent())) {
+        if (noPropertiesFileArgument != null && noPropertiesFileArgument.isPresent()) {
             return null;
         }
 
@@ -516,8 +535,7 @@
             }
         } catch (final Exception e) {
             final LocalizableMessage message =
-                    ERR_ARGPARSER_CANNOT_READ_PROPERTIES_FILE.get(String
-                            .valueOf(propertiesFilePath), getExceptionMessage(e));
+                    ERR_ARGPARSER_CANNOT_READ_PROPERTIES_FILE.get(propertiesFilePath, getExceptionMessage(e));
             throw new ArgumentException(message, e);
         }
         return argumentProperties;
@@ -727,23 +745,23 @@
     void getUsage(final StringBuilder buffer) {
         usageOrVersionDisplayed = true;
         final String scriptName = System.getProperty(PROPERTY_SCRIPT_NAME);
-        if ((scriptName == null) || (scriptName.length() == 0)) {
+        if (scriptName == null || scriptName.length() == 0) {
             buffer.append(INFO_ARGPARSER_USAGE_JAVA_CLASSNAME.get(mainClassName));
         } else {
             buffer.append(INFO_ARGPARSER_USAGE_JAVA_SCRIPTNAME.get(scriptName));
         }
 
         if (allowsTrailingArguments) {
+            buffer.append(" ");
             if (trailingArgsDisplayName == null) {
-                buffer.append(" " + INFO_ARGPARSER_USAGE_TRAILINGARGS.get());
+                buffer.append(INFO_ARGPARSER_USAGE_TRAILINGARGS.get());
             } else {
-                buffer.append(" ");
                 buffer.append(trailingArgsDisplayName);
             }
         }
         buffer.append(EOL);
         buffer.append(EOL);
-        if ((toolDescription != null) && (toolDescription.length() > 0)) {
+        if (toolDescription != null && toolDescription.length() > 0) {
             buffer.append(wrapText(toolDescription.toString(), MAX_LENGTH - 1));
             buffer.append(EOL);
             buffer.append(EOL);
@@ -769,9 +787,8 @@
 
             final SortedSet<Argument> args = new TreeSet<Argument>(new Comparator<Argument>() {
 
-                /**
-                 * {@inheritDoc}
-                 */
+                /** {@inheritDoc} */
+                @Override
                 public int compare(final Argument o1, final Argument o2) {
                     final String s1;
                     final String s2;
@@ -807,7 +824,7 @@
                 }
 
                 // Help argument should be printed at the end
-                if ((usageArgument != null) && usageArgument.getName().equals(a.getName())) {
+                if (usageArgument != null && usageArgument.getName().equals(a.getName())) {
                     helpArgument = a;
                     continue;
                 }
@@ -847,11 +864,7 @@
      *         <CODE>false</CODE> otherwise.
      */
     public boolean isUsageArgumentPresent() {
-        boolean isUsageArgumentPresent = false;
-        if (usageArgument != null) {
-            isUsageArgumentPresent = usageArgument.isPresent();
-        }
-        return isUsageArgumentPresent;
+        return usageArgument != null && usageArgument.isPresent();
     }
 
     /**
@@ -905,7 +918,7 @@
 
             if (inTrailingArgs) {
                 trailingArguments.add(arg);
-                if ((maxTrailingArguments > 0) && (trailingArguments.size() > maxTrailingArguments)) {
+                if (maxTrailingArguments > 0 && trailingArguments.size() > maxTrailingArguments) {
                     final LocalizableMessage message =
                             ERR_ARGPARSER_TOO_MANY_TRAILING_ARGS.get(maxTrailingArguments);
                     throw new ArgumentException(message);
@@ -989,7 +1002,7 @@
 
                     // If this is the usage argument, then immediately stop and
                     // print usage information.
-                    if ((usageArgument != null) && usageArgument.getName().equals(a.getName())) {
+                    if (usageArgument != null && usageArgument.getName().equals(a.getName())) {
                         try {
                             getUsage(usageOutputStream);
                         } catch (final Exception e) {
@@ -1024,10 +1037,8 @@
 
                     // If the argument already has a value, then make sure it is
                     // acceptable to have more than one.
-                    if (a.hasValue() && (!a.isMultiValued())) {
-                        final LocalizableMessage message =
-                                ERR_ARGPARSER_NOT_MULTIVALUED_FOR_LONG_ID.get(origArgName);
-                        throw new ArgumentException(message);
+                    if (a.hasValue() && !a.isMultiValued()) {
+                        throw new ArgumentException(ERR_ARGPARSER_NOT_MULTIVALUED_FOR_LONG_ID.get(origArgName));
                     }
 
                     a.addValue(argValue);
@@ -1046,8 +1057,7 @@
                 // -nvalue
                 // -n value
                 if (arg.equals("-")) {
-                    final LocalizableMessage message = ERR_ARGPARSER_INVALID_DASH_AS_ARGUMENT.get();
-                    throw new ArgumentException(message);
+                    throw new ArgumentException(ERR_ARGPARSER_INVALID_DASH_AS_ARGUMENT.get());
                 }
 
                 final char argCharacter = arg.charAt(1);
@@ -1071,8 +1081,8 @@
                         }
 
                         return;
-                    } else if ((argCharacter == OPTION_SHORT_PRODUCT_VERSION)
-                            && (!shortIDMap.containsKey(OPTION_SHORT_PRODUCT_VERSION))) {
+                    } else if (argCharacter == OPTION_SHORT_PRODUCT_VERSION
+                            && !shortIDMap.containsKey(OPTION_SHORT_PRODUCT_VERSION)) {
                         // "-V" will always be interpreted as requesting
                         // version information except if it's already defined
                         // (e.g
@@ -1088,17 +1098,14 @@
                         return;
                     } else {
                         // There is no such argument registered.
-                        final LocalizableMessage message =
-                                ERR_ARGPARSER_NO_ARGUMENT_WITH_SHORT_ID.get(String
-                                        .valueOf(argCharacter));
-                        throw new ArgumentException(message);
+                        throw new ArgumentException(ERR_ARGPARSER_NO_ARGUMENT_WITH_SHORT_ID.get(argCharacter));
                     }
                 } else {
                     a.setPresent(true);
 
                     // If this is the usage argument, then immediately stop and
                     // print usage information.
-                    if ((usageArgument != null) && usageArgument.getName().equals(a.getName())) {
+                    if (usageArgument != null && usageArgument.getName().equals(a.getName())) {
                         try {
                             getUsage(usageOutputStream);
                         } catch (final Exception e) {
@@ -1114,10 +1121,8 @@
                 if (a.needsValue()) {
                     if (argValue == null) {
                         if ((i + 1) == numArguments) {
-                            final LocalizableMessage message =
-                                    ERR_ARGPARSER_NO_VALUE_FOR_ARGUMENT_WITH_SHORT_ID.get(String
-                                            .valueOf(argCharacter));
-                            throw new ArgumentException(message);
+                            throw new ArgumentException(
+                                    ERR_ARGPARSER_NO_VALUE_FOR_ARGUMENT_WITH_SHORT_ID.get(argCharacter));
                         }
 
                         argValue = rawArguments[++i];
@@ -1125,33 +1130,23 @@
 
                     final LocalizableMessageBuilder invalidReason = new LocalizableMessageBuilder();
                     if (!a.valueIsAcceptable(argValue, invalidReason)) {
-                        final LocalizableMessage message =
-                                ERR_ARGPARSER_VALUE_UNACCEPTABLE_FOR_SHORT_ID.get(argValue, String
-                                        .valueOf(argCharacter), invalidReason.toString());
-                        throw new ArgumentException(message);
+                        throw new ArgumentException(ERR_ARGPARSER_VALUE_UNACCEPTABLE_FOR_SHORT_ID.get(
+                                argValue, argCharacter, invalidReason));
                     }
 
                     // If the argument already has a value, then make sure it is
                     // acceptable to have more than one.
-                    if (a.hasValue() && (!a.isMultiValued())) {
-                        final LocalizableMessage message =
-                                ERR_ARGPARSER_NOT_MULTIVALUED_FOR_SHORT_ID.get(String
-                                        .valueOf(argCharacter));
-                        throw new ArgumentException(message);
+                    if (a.hasValue() && !a.isMultiValued()) {
+                        throw new ArgumentException(ERR_ARGPARSER_NOT_MULTIVALUED_FOR_SHORT_ID.get(argCharacter));
                     }
 
                     a.addValue(argValue);
                 } else {
                     if (argValue != null) {
-                        // If we've gotten here, then it means that we're in a
-                        // scenario like
-                        // "-abc" where "a" is a valid argument that doesn't
-                        // take a
-                        // value. However, this could still be valid if all
-                        // remaining
-                        // characters in the value are also valid argument
-                        // characters that
-                        // don't take values.
+                        // If we've gotten here, then it means that we're in a scenario like
+                        // "-abc" where "a" is a valid argument that doesn't take a value.
+                        // However, this could still be valid if all remaining characters
+                        // in the value are also valid argument characters that don't take values.
                         final int valueLength = argValue.length();
                         for (int j = 0; j < valueLength; j++) {
                             final char c = argValue.charAt(j);
@@ -1159,28 +1154,21 @@
                             if (b == null) {
                                 // There is no such argument registered.
                                 final LocalizableMessage message =
-                                        ERR_ARGPARSER_NO_ARGUMENT_WITH_SHORT_ID.get(String
-                                                .valueOf(argCharacter));
+                                        ERR_ARGPARSER_NO_ARGUMENT_WITH_SHORT_ID.get(argCharacter);
                                 throw new ArgumentException(message);
                             } else if (b.needsValue()) {
                                 // This means we're in a scenario like "-abc"
-                                // where b is
-                                // a valid argument that takes a value. We don't
-                                // support
-                                // that.
+                                // where b is a valid argument that takes a value.
+                                // We don't support that.
                                 final LocalizableMessage message =
-                                        ERR_ARGPARSER_CANT_MIX_ARGS_WITH_VALUES
-                                                .get(String.valueOf(argCharacter), argValue, String
-                                                        .valueOf(c));
+                                        ERR_ARGPARSER_CANT_MIX_ARGS_WITH_VALUES.get(argCharacter, argValue, c);
                                 throw new ArgumentException(message);
                             } else {
                                 b.setPresent(true);
 
-                                // If this is the usage argument, then
-                                // immediately stop
-                                // and print usage information.
-                                if ((usageArgument != null)
-                                        && usageArgument.getName().equals(b.getName())) {
+                                // If this is the usage argument,
+                                // then immediately stop and print usage information.
+                                if (usageArgument != null && usageArgument.getName().equals(b.getName())) {
                                     try {
                                         getUsage(usageOutputStream);
                                     } catch (final Exception e) {
@@ -1201,20 +1189,16 @@
             } else {
                 // It doesn't start with a dash and we don't allow trailing
                 // arguments, so this is illegal.
-                final LocalizableMessage message =
-                        ERR_ARGPARSER_DISALLOWED_TRAILING_ARGUMENT.get(arg);
-                throw new ArgumentException(message);
+                throw new ArgumentException(ERR_ARGPARSER_DISALLOWED_TRAILING_ARGUMENT.get(arg));
             }
         }
 
         // If we allow trailing arguments and there is a minimum number,
         // then make sure at least that many were provided.
-        if (allowsTrailingArguments && (minTrailingArguments > 0)) {
-            if (trailingArguments.size() < minTrailingArguments) {
-                final LocalizableMessage message =
-                        ERR_ARGPARSER_TOO_FEW_TRAILING_ARGUMENTS.get(minTrailingArguments);
-                throw new ArgumentException(message);
-            }
+        if (allowsTrailingArguments
+                && minTrailingArguments > 0
+                && trailingArguments.size() < minTrailingArguments) {
+            throw new ArgumentException(ERR_ARGPARSER_TOO_FEW_TRAILING_ARGUMENTS.get(minTrailingArguments));
         }
 
         // If we don't have the argumentProperties, try to load a properties
@@ -1223,49 +1207,11 @@
             argumentProperties = checkExternalProperties();
         }
 
-        // Iterate through all of the arguments. For any that were not
-        // provided on the command line, see if there is an alternate default
-        // that
-        // can be used. For cases where there is not, see that argument is
-        // required.
-        for (final Argument a : argumentList) {
-            if (!a.isPresent()) {
-                // See if there is a value in the properties that can be used
-                if ((argumentProperties != null) && (a.getPropertyName() != null)) {
-                    final String value =
-                            argumentProperties.getProperty(a.getPropertyName().toLowerCase());
-                    final LocalizableMessageBuilder invalidReason = new LocalizableMessageBuilder();
-                    if (value != null) {
-                        Boolean addValue = true;
-                        if (!(a instanceof BooleanArgument)) {
-                            addValue = a.valueIsAcceptable(value, invalidReason);
-                        }
-                        if (addValue) {
-                            a.addValue(value);
-                            if (a.needsValue()) {
-                                a.setPresent(true);
-                            }
-                            a.setValueSetByProperty(true);
-                        }
-                    }
-                }
-            }
-
-            if ((!a.isPresent()) && a.needsValue()) {
-                // See if the argument defines a default.
-                if (a.getDefaultValue() != null) {
-                    a.addValue(a.getDefaultValue());
-                }
-
-                // If there is still no value and the argument is required, then
-                // that's a problem.
-                if ((!a.hasValue()) && a.isRequired()) {
-                    final LocalizableMessage message =
-                            ERR_ARGPARSER_NO_VALUE_FOR_REQUIRED_ARG.get(a.getName());
-                    throw new ArgumentException(message);
-                }
-            }
-        }
+        // Iterate through all of the arguments.
+        // For any that were not provided on the command line,
+        // see if there is an alternate default that can be used.
+        // For cases where there is not, see that argument is required.
+        normalizeArguments(argumentProperties, argumentList);
     }
 
     /**
@@ -1459,9 +1405,8 @@
                         + DEFAULT_OPENDJ_PROPERTIES_FILE_EXTENSION);
         if (f.exists() && f.canRead()) {
             return f.getAbsolutePath();
-        } else {
-            return null;
         }
+        return null;
     }
 
     private void initGroups() {
@@ -1489,11 +1434,10 @@
     }
 
     private boolean isInputOutputArgument(final Argument arg) {
-        boolean io = false;
         if (arg != null) {
             final String longId = arg.getLongIdentifier();
-            io =
-                    OPTION_LONG_VERBOSE.equals(longId) || OPTION_LONG_QUIET.equals(longId)
+            return OPTION_LONG_VERBOSE.equals(longId)
+                            || OPTION_LONG_QUIET.equals(longId)
                             || OPTION_LONG_NO_PROMPT.equals(longId)
                             || OPTION_LONG_PROP_FILE_PATH.equals(longId)
                             || OPTION_LONG_NO_PROP_FILE.equals(longId)
@@ -1502,16 +1446,16 @@
                             || OPTION_LONG_ENCODING.equals(longId)
                             || OPTION_LONG_BATCH_FILE_PATH.equals(longId);
         }
-        return io;
+        return false;
     }
 
     private boolean isLdapConnectionArgument(final Argument arg) {
-        boolean ldap = false;
         if (arg != null) {
             final String longId = arg.getLongIdentifier();
-            ldap =
-                    OPTION_LONG_USE_SSL.equals(longId) || OPTION_LONG_START_TLS.equals(longId)
-                            || OPTION_LONG_HOST.equals(longId) || OPTION_LONG_PORT.equals(longId)
+            return OPTION_LONG_USE_SSL.equals(longId)
+                            || OPTION_LONG_START_TLS.equals(longId)
+                            || OPTION_LONG_HOST.equals(longId)
+                            || OPTION_LONG_PORT.equals(longId)
                             || OPTION_LONG_BINDDN.equals(longId)
                             || OPTION_LONG_BINDPWD.equals(longId)
                             || OPTION_LONG_BINDPWD_FILE.equals(longId)
@@ -1531,7 +1475,7 @@
                             || OPTION_LONG_USE_SASL_EXTERNAL.equals(longId)
                             || OPTION_LONG_PROTOCOL_VERSION.equals(longId);
         }
-        return ldap;
+        return false;
     }
 
     /**
@@ -1544,8 +1488,7 @@
      */
     private void printArgumentUsage(final Argument a, final StringBuilder buffer) {
         // Write a line with the short and/or long identifiers that may be
-        // used
-        // for the argument.
+        // used for the argument.
         final int indentLength = INDENT.length();
         final Character shortID = a.getShortIdentifier();
         final String longID = a.getLongIdentifier();
@@ -1577,10 +1520,8 @@
                 final int lineLength = (buffer.length() - currentLength) + newBuffer.length();
                 if (lineLength > MAX_LENGTH) {
                     buffer.append(EOL);
-                    buffer.append(newBuffer.toString());
-                } else {
-                    buffer.append(newBuffer.toString());
                 }
+                buffer.append(newBuffer);
             }
 
             buffer.append(EOL);
@@ -1607,7 +1548,7 @@
         buffer.append(wrapText(a.getDescription(), MAX_LENGTH, indentLength));
         buffer.append(EOL);
 
-        if (a.needsValue() && (a.getDefaultValue() != null) && (a.getDefaultValue().length() > 0)) {
+        if (a.needsValue() && a.getDefaultValue() != null && a.getDefaultValue().length() > 0) {
             buffer.append(INDENT);
             buffer.append(INFO_ARGPARSER_USAGE_DEFAULT_VALUE.get(a.getDefaultValue()).toString());
             buffer.append(EOL);
diff --git a/opendj-cli/src/main/java/com/forgerock/opendj/cli/CommandBuilder.java b/opendj-cli/src/main/java/com/forgerock/opendj/cli/CommandBuilder.java
index a4da3f1..b2a28ef 100644
--- a/opendj-cli/src/main/java/com/forgerock/opendj/cli/CommandBuilder.java
+++ b/opendj-cli/src/main/java/com/forgerock/opendj/cli/CommandBuilder.java
@@ -28,9 +28,12 @@
 
 import static com.forgerock.opendj.cli.Utils.OBFUSCATED_VALUE;
 
+import java.util.Arrays;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Set;
+import java.util.TreeSet;
 
 import com.forgerock.opendj.util.OperatingSystem;
 
@@ -188,28 +191,25 @@
             } else if (arg instanceof FileBasedArgument) {
                 for (String value : ((FileBasedArgument) arg).getNameToValueMap().keySet()) {
                     builder.append(lineSeparator + argName + " ");
-                    if (isObfuscated(arg) && !showObfuscated) {
-                        value = OBFUSCATED_VALUE;
-                    } else {
-                        value = escapeValue(value);
-                    }
-                    builder.append(value);
+                    builder.append(getOutputValue(value, arg, showObfuscated));
                 }
             } else {
                 for (String value : arg.getValues()) {
                     builder.append(lineSeparator + argName + " ");
-                    if (isObfuscated(arg) && !showObfuscated) {
-                        value = OBFUSCATED_VALUE;
-                    } else {
-                        value = escapeValue(value);
-                    }
-                    builder.append(value);
+                    builder.append(getOutputValue(value, arg, showObfuscated));
                 }
             }
         }
         return builder.toString();
     }
 
+    private String getOutputValue(final String value, final Argument arg, final boolean showObfuscated) {
+        if (isObfuscated(arg) && !showObfuscated) {
+            return OBFUSCATED_VALUE;
+        }
+        return escapeValue(value);
+    }
+
     /**
      * Clears the arguments.
      */
@@ -238,9 +238,9 @@
         return obfuscatedArgs.contains(argument);
     }
 
-    // Chars that require special treatment when passing them to command-line.
-    private final static char[] CHARSTOESCAPE =
-    { ' ', '\t', '\n', '|', ';', '<', '>', '(', ')', '$', '`', '\\', '"', '\'' };
+    /** Chars that require special treatment when passing them to command-line. */
+    private final static Set<Character> CHARSTOESCAPE = new TreeSet<Character>(Arrays.asList(
+        ' ', '\t', '\n', '|', ';', '<', '>', '(', ')', '$', '`', '\\', '"', '\''));
 
     /**
      * This method simply takes a value and tries to transform it (with escape or '"') characters so that it can be used
@@ -255,11 +255,7 @@
         if (OperatingSystem.isUnix()) {
             for (int i = 0; i < value.length(); i++) {
                 final char c = value.charAt(i);
-                boolean charToEscapeFound = false;
-                for (int j = 0; j < CHARSTOESCAPE.length && !charToEscapeFound; j++) {
-                    charToEscapeFound = c == CHARSTOESCAPE[j];
-                }
-                if (charToEscapeFound) {
+                if (CHARSTOESCAPE.contains(c)) {
                     b.append('\\');
                 }
                 b.append(c);
diff --git a/opendj-cli/src/main/java/com/forgerock/opendj/cli/CommonArguments.java b/opendj-cli/src/main/java/com/forgerock/opendj/cli/CommonArguments.java
index f400649..4ca01da 100644
--- a/opendj-cli/src/main/java/com/forgerock/opendj/cli/CommonArguments.java
+++ b/opendj-cli/src/main/java/com/forgerock/opendj/cli/CommonArguments.java
@@ -45,7 +45,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getShowUsage() throws ArgumentException {
+    public static BooleanArgument getShowUsage() throws ArgumentException {
         return new BooleanArgument("showUsage", OPTION_SHORT_HELP, OPTION_LONG_HELP, INFO_DESCRIPTION_SHOWUSAGE.get());
     }
 
@@ -56,7 +56,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getVerbose() throws ArgumentException {
+    public static BooleanArgument getVerbose() throws ArgumentException {
         final BooleanArgument verbose = new BooleanArgument("verbose", 'v', "verbose", INFO_DESCRIPTION_VERBOSE.get());
         verbose.setPropertyName("verbose");
         return verbose;
@@ -69,7 +69,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getPropertiesFile() throws ArgumentException {
+    public static StringArgument getPropertiesFile() throws ArgumentException {
         return new StringArgument("propertiesFilePath", null, OPTION_LONG_PROP_FILE_PATH, false, false, true,
                 INFO_PROP_FILE_PATH_PLACEHOLDER.get(), null, null, INFO_DESCRIPTION_PROP_FILE_PATH.get());
     }
@@ -81,7 +81,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getNoPropertiesFile() throws ArgumentException {
+    public static BooleanArgument getNoPropertiesFile() throws ArgumentException {
         return new BooleanArgument("noPropertiesFile", null, OPTION_LONG_NO_PROP_FILE,
                 INFO_DESCRIPTION_NO_PROP_FILE.get());
     }
@@ -93,7 +93,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getContinueOnError() throws ArgumentException {
+    public static BooleanArgument getContinueOnError() throws ArgumentException {
         final BooleanArgument continueOnError = new BooleanArgument("continueOnError", 'c', "continueOnError",
                 INFO_DESCRIPTION_CONTINUE_ON_ERROR.get());
         continueOnError.setPropertyName("continueOnError");
@@ -107,7 +107,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final IntegerArgument getVersion() throws ArgumentException {
+    public static IntegerArgument getVersion() throws ArgumentException {
         final IntegerArgument version = new IntegerArgument("version", OPTION_SHORT_PROTOCOL_VERSION,
                 OPTION_LONG_PROTOCOL_VERSION, false, false, true, INFO_PROTOCOL_VERSION_PLACEHOLDER.get(), 3, null,
                 INFO_DESCRIPTION_VERSION.get());
@@ -122,7 +122,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getQuiet() throws ArgumentException {
+    public static BooleanArgument getQuiet() throws ArgumentException {
         final BooleanArgument quiet = new BooleanArgument(OPTION_LONG_QUIET, OPTION_SHORT_QUIET, OPTION_LONG_QUIET,
                 INFO_DESCRIPTION_QUIET.get());
         quiet.setPropertyName(OPTION_LONG_QUIET);
@@ -137,7 +137,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getNoPrompt() throws ArgumentException {
+    public static BooleanArgument getNoPrompt() throws ArgumentException {
         return new BooleanArgument(OPTION_LONG_NO_PROMPT, OPTION_SHORT_NO_PROMPT, OPTION_LONG_NO_PROMPT,
                 INFO_DESCRIPTION_NO_PROMPT.get());
     }
@@ -149,7 +149,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getAcceptLicense() throws ArgumentException {
+    public static BooleanArgument getAcceptLicense() throws ArgumentException {
         return new BooleanArgument(OPTION_LONG_ACCEPT_LICENSE, null, OPTION_LONG_ACCEPT_LICENSE,
                 INFO_OPTION_ACCEPT_LICENSE.get());
     }
@@ -161,7 +161,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getTestOnly() throws ArgumentException {
+    public static BooleanArgument getTestOnly() throws ArgumentException {
         final BooleanArgument testOnly = new BooleanArgument("testOnly".toLowerCase(), 't', "testOnly",
                 INFO_ARGUMENT_DESCRIPTION_TESTONLY.get());
         testOnly.setHidden(true);
@@ -178,7 +178,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final IntegerArgument getConnectTimeOut(final int defaultTimeout) throws ArgumentException {
+    public static IntegerArgument getConnectTimeOut(final int defaultTimeout) throws ArgumentException {
         final IntegerArgument connectTimeout = new IntegerArgument(OPTION_LONG_CONNECT_TIMEOUT, null,
                 OPTION_LONG_CONNECT_TIMEOUT, false, false, true, INFO_TIMEOUT_PLACEHOLDER.get(), defaultTimeout, null,
                 true, 1, true, 65535, INFO_DESCRIPTION_CONNECTION_TIMEOUT.get());
@@ -194,7 +194,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getCLI() throws ArgumentException {
+    public static BooleanArgument getCLI() throws ArgumentException {
         final BooleanArgument cli = new BooleanArgument(OPTION_LONG_CLI.toLowerCase(), OPTION_SHORT_CLI,
                 OPTION_LONG_CLI, INFO_ARGUMENT_DESCRIPTION_CLI.get());
         cli.setPropertyName(OPTION_LONG_CLI);
@@ -208,7 +208,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getBaseDN() throws ArgumentException {
+    public static StringArgument getBaseDN() throws ArgumentException {
         return new StringArgument(OPTION_LONG_BASEDN.toLowerCase(), OPTION_SHORT_BASEDN, OPTION_LONG_BASEDN, false,
                 true, true, INFO_BASEDN_PLACEHOLDER.get(), null, OPTION_LONG_BASEDN,
                 INFO_ARGUMENT_DESCRIPTION_BASEDN.get());
@@ -221,7 +221,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getAddBaseEntry() throws ArgumentException {
+    public static BooleanArgument getAddBaseEntry() throws ArgumentException {
         final BooleanArgument addBaseEntryArg = new BooleanArgument("addBaseEntry".toLowerCase(), 'a', "addBaseEntry",
                 INFO_ARGUMENT_DESCRIPTION_ADDBASE.get());
         addBaseEntryArg.setPropertyName("addBaseEntry");
@@ -235,7 +235,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getImportLDIF() throws ArgumentException {
+    public static StringArgument getImportLDIF() throws ArgumentException {
         return new StringArgument(OPTION_LONG_LDIF_FILE.toLowerCase(), OPTION_SHORT_LDIF_FILE, OPTION_LONG_LDIF_FILE,
                 false, true, true, INFO_LDIFFILE_PLACEHOLDER.get(), null, OPTION_LONG_LDIF_FILE,
                 INFO_ARGUMENT_DESCRIPTION_IMPORTLDIF.get());
@@ -248,7 +248,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getRejectedImportLdif() throws ArgumentException {
+    public static StringArgument getRejectedImportLdif() throws ArgumentException {
         return new StringArgument("rejectFile".toLowerCase(), 'R', "rejectFile", false, false, true,
                 INFO_REJECT_FILE_PLACEHOLDER.get(), null, "rejectFile", INFO_GENERAL_DESCRIPTION_REJECTED_FILE.get());
     }
@@ -260,7 +260,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getSkippedImportFile() throws ArgumentException {
+    public static StringArgument getSkippedImportFile() throws ArgumentException {
         return new StringArgument("skipFile".toLowerCase(), null, "skipFile", false, false, true,
                 INFO_SKIP_FILE_PLACEHOLDER.get(), null, "skipFile", INFO_GENERAL_DESCRIPTION_SKIPPED_FILE.get());
     }
@@ -272,7 +272,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final IntegerArgument getSampleData() throws ArgumentException {
+    public static IntegerArgument getSampleData() throws ArgumentException {
         return new IntegerArgument("sampleData".toLowerCase(), 'd', "sampleData", false, false, true,
                 INFO_NUM_ENTRIES_PLACEHOLDER.get(), 0, "sampleData", true, 0, false, 0,
                 INFO_SETUP_DESCRIPTION_SAMPLE_DATA.get());
@@ -287,7 +287,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final IntegerArgument getLDAPPort(final int defaultLdapPort) throws ArgumentException {
+    public static IntegerArgument getLDAPPort(final int defaultLdapPort) throws ArgumentException {
         return new IntegerArgument("ldapPort".toLowerCase(), OPTION_SHORT_PORT, "ldapPort", false, false, true,
                 INFO_PORT_PLACEHOLDER.get(), defaultLdapPort, "ldapPort", true, 1, true, 65535,
                 INFO_ARGUMENT_DESCRIPTION_LDAPPORT.get());
@@ -302,7 +302,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final IntegerArgument getAdminLDAPPort(final int defaultAdminPort) throws ArgumentException {
+    public static IntegerArgument getAdminLDAPPort(final int defaultAdminPort) throws ArgumentException {
         return new IntegerArgument("adminConnectorPort".toLowerCase(), null, "adminConnectorPort", false, false, true,
                 INFO_PORT_PLACEHOLDER.get(), defaultAdminPort, "adminConnectorPort", true, 1, true, 65535,
                 INFO_ARGUMENT_DESCRIPTION_ADMINCONNECTORPORT.get());
@@ -317,7 +317,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final IntegerArgument getJMXPort(final int defaultJMXPort) throws ArgumentException {
+    public static IntegerArgument getJMXPort(final int defaultJMXPort) throws ArgumentException {
         return new IntegerArgument("jmxPort".toLowerCase(), 'x', "jmxPort", false, false, true,
                 INFO_JMXPORT_PLACEHOLDER.get(), defaultJMXPort, "jmxPort", true, 1, true, 65535,
                 INFO_ARGUMENT_DESCRIPTION_SKIPPORT.get());
@@ -330,7 +330,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getSkipPortCheck() throws ArgumentException {
+    public static BooleanArgument getSkipPortCheck() throws ArgumentException {
         final BooleanArgument skipPortCheck = new BooleanArgument("skipPortCheck".toLowerCase(), 'S', "skipPortCheck",
                 INFO_ARGUMENT_DESCRIPTION_SKIPPORT.get());
         skipPortCheck.setPropertyName("skipPortCheck");
@@ -344,7 +344,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getRootDN() throws ArgumentException {
+    public static StringArgument getRootDN() throws ArgumentException {
         return new StringArgument(OPTION_LONG_ROOT_USER_DN.toLowerCase(), OPTION_SHORT_ROOT_USER_DN,
                 OPTION_LONG_ROOT_USER_DN, false, false, true, INFO_ROOT_USER_DN_PLACEHOLDER.get(),
                 "cn=Directory Manager", OPTION_LONG_ROOT_USER_DN, INFO_ARGUMENT_DESCRIPTION_ROOTDN.get());
@@ -357,7 +357,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getRootDNPwd() throws ArgumentException {
+    public static StringArgument getRootDNPwd() throws ArgumentException {
         return new StringArgument("rootUserPassword".toLowerCase(), OPTION_SHORT_BINDPWD, "rootUserPassword", false,
                 false, true, INFO_ROOT_USER_PWD_PLACEHOLDER.get(), null, "rootUserPassword",
                 INFO_ROOT_USER_PWD_PLACEHOLDER.get());
@@ -370,7 +370,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final FileBasedArgument getRootDNPwdFile() throws ArgumentException {
+    public static FileBasedArgument getRootDNPwdFile() throws ArgumentException {
         return new FileBasedArgument("rootUserPasswordFile".toLowerCase(), OPTION_SHORT_BINDPWD_FILE,
                 "rootUserPasswordFile", false, false, INFO_ROOT_USER_PWD_FILE_PLACEHOLDER.get(), null,
                 "rootUserPasswordFile", INFO_ARGUMENT_DESCRIPTION_ROOTPWFILE.get());
@@ -383,7 +383,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getEnableWindowsService() throws ArgumentException {
+    public static BooleanArgument getEnableWindowsService() throws ArgumentException {
         final BooleanArgument enableWindowsServiceArg = new BooleanArgument("enableWindowsService".toLowerCase(), 'e',
                 "enableWindowsService", INFO_ARGUMENT_DESCRIPTION_ENABLE_WINDOWS_SERVICE.get());
         enableWindowsServiceArg.setPropertyName("enableWindowsService");
@@ -397,7 +397,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getDoNotStart() throws ArgumentException {
+    public static BooleanArgument getDoNotStart() throws ArgumentException {
         final BooleanArgument doNotStartArg = new BooleanArgument("doNotStart".toLowerCase(), 'O', "doNotStart",
                 INFO_SETUP_DESCRIPTION_DO_NOT_START.get());
         doNotStartArg.setPropertyName("doNotStart");
@@ -411,7 +411,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getEnableTLS() throws ArgumentException {
+    public static BooleanArgument getEnableTLS() throws ArgumentException {
         final BooleanArgument enableStartTLS = new BooleanArgument("enableStartTLS".toLowerCase(),
                 OPTION_SHORT_START_TLS, "enableStartTLS", INFO_SETUP_DESCRIPTION_ENABLE_STARTTLS.get());
         enableStartTLS.setPropertyName("enableStartTLS");
@@ -427,7 +427,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final IntegerArgument getLDAPSPort(final int defaultSecurePort) throws ArgumentException {
+    public static IntegerArgument getLDAPSPort(final int defaultSecurePort) throws ArgumentException {
         return new IntegerArgument("ldapsPort".toLowerCase(), OPTION_SHORT_USE_SSL, "ldapsPort", false, false, true,
                 INFO_PORT_PLACEHOLDER.get(), defaultSecurePort, "ldapsPort", true, 1, true, 65535,
                 INFO_ARGUMENT_DESCRIPTION_LDAPSPORT.get());
@@ -440,7 +440,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getGenerateSelfSigned() throws ArgumentException {
+    public static BooleanArgument getGenerateSelfSigned() throws ArgumentException {
         final BooleanArgument generateSelfSigned = new BooleanArgument("generateSelfSignedCertificate".toLowerCase(),
                 null, "generateSelfSignedCertificate", INFO_ARGUMENT_DESCRIPTION_USE_SELF_SIGNED_CERTIFICATE.get());
         generateSelfSigned.setPropertyName("generateSelfSignedCertificate");
@@ -456,7 +456,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getHostName(final String defaultHostName) throws ArgumentException {
+    public static StringArgument getHostName(final String defaultHostName) throws ArgumentException {
         final StringArgument hostName = new StringArgument(OPTION_LONG_HOST.toLowerCase(), OPTION_SHORT_HOST,
                 OPTION_LONG_HOST, false, false, true, INFO_HOST_PLACEHOLDER.get(), defaultHostName, null,
                 INFO_ARGUMENT_DESCRIPTION_HOST_NAME.get());
@@ -471,7 +471,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final BooleanArgument getUsePKCS11Keystore() throws ArgumentException {
+    public static BooleanArgument getUsePKCS11Keystore() throws ArgumentException {
         final BooleanArgument usePkcs11 = new BooleanArgument("usePkcs11Keystore".toLowerCase(), null,
                 "usePkcs11Keystore", INFO_ARGUMENT_DESCRIPTION_USE_PKCS11.get());
         usePkcs11.setPropertyName("usePkcs11Keystore");
@@ -485,7 +485,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getUseJavaKeyStore() throws ArgumentException {
+    public static StringArgument getUseJavaKeyStore() throws ArgumentException {
         return new StringArgument("useJavaKeystore".toLowerCase(), null, "useJavaKeystore", false, false, true,
                 INFO_KEYSTOREPATH_PLACEHOLDER.get(), null, "useJavaKeystore",
                 INFO_ARGUMENT_DESCRIPTION_USE_JAVAKEYSTORE.get());
@@ -498,7 +498,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getUseJCEKS() throws ArgumentException {
+    public static StringArgument getUseJCEKS() throws ArgumentException {
         return new StringArgument("useJCEKS".toLowerCase(), null, "useJCEKS", false, false, true,
                 INFO_KEYSTOREPATH_PLACEHOLDER.get(), null, "useJCEKS", INFO_ARGUMENT_DESCRIPTION_USE_JCEKS.get());
     }
@@ -510,7 +510,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getUsePKCS12KeyStore() throws ArgumentException {
+    public static StringArgument getUsePKCS12KeyStore() throws ArgumentException {
         return new StringArgument("usePkcs12keyStore".toLowerCase(), null, "usePkcs12keyStore", false, false, true,
                 INFO_KEYSTOREPATH_PLACEHOLDER.get(), null, "usePkcs12keyStore",
                 INFO_ARGUMENT_DESCRIPTION_USE_PKCS12.get());
@@ -523,7 +523,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getKeyStorePassword() throws ArgumentException {
+    public static StringArgument getKeyStorePassword() throws ArgumentException {
         return new StringArgument(OPTION_LONG_KEYSTORE_PWD.toLowerCase(), OPTION_SHORT_KEYSTORE_PWD,
                 OPTION_LONG_KEYSTORE_PWD, false, false, true, INFO_KEYSTORE_PWD_PLACEHOLDER.get(), null,
                 OPTION_LONG_KEYSTORE_PWD, INFO_ARGUMENT_DESCRIPTION_KEYSTOREPASSWORD.get());
@@ -536,7 +536,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final FileBasedArgument getKeyStorePasswordFile() throws ArgumentException {
+    public static FileBasedArgument getKeyStorePasswordFile() throws ArgumentException {
         return new FileBasedArgument(OPTION_LONG_KEYSTORE_PWD_FILE.toLowerCase(), OPTION_SHORT_KEYSTORE_PWD_FILE,
                 OPTION_LONG_KEYSTORE_PWD_FILE, false, false, INFO_KEYSTORE_PWD_FILE_PLACEHOLDER.get(), null,
                 OPTION_LONG_KEYSTORE_PWD_FILE, INFO_ARGUMENT_DESCRIPTION_KEYSTOREPASSWORD_FILE.get());
@@ -549,7 +549,7 @@
      * @throws ArgumentException
      *             If there is a problem with any of the parameters used to create this argument.
      */
-    public static final StringArgument getCertNickName() throws ArgumentException {
+    public static StringArgument getCertNickName() throws ArgumentException {
         return new StringArgument(OPTION_LONG_CERT_NICKNAME.toLowerCase(), OPTION_SHORT_CERT_NICKNAME,
                 OPTION_LONG_CERT_NICKNAME, false, false, true, INFO_NICKNAME_PLACEHOLDER.get(), null,
                 OPTION_LONG_CERT_NICKNAME, INFO_ARGUMENT_DESCRIPTION_CERT_NICKNAME.get());
diff --git a/opendj-cli/src/main/java/com/forgerock/opendj/cli/MultiColumnPrinter.java b/opendj-cli/src/main/java/com/forgerock/opendj/cli/MultiColumnPrinter.java
index c25f19d..9ad2138 100644
--- a/opendj-cli/src/main/java/com/forgerock/opendj/cli/MultiColumnPrinter.java
+++ b/opendj-cli/src/main/java/com/forgerock/opendj/cli/MultiColumnPrinter.java
@@ -125,17 +125,17 @@
     /**
      * Left ID.
      */
-    static public final int LEFT = 0;
+    public static final int LEFT = 0;
 
     /**
      * Center ID.
      */
-    static public final int CENTER = 1;
+    public static final int CENTER = 1;
 
     /**
      * Right ID.
      */
-    static public final int RIGHT = 2;
+    public static final int RIGHT = 2;
 
     private int numCol = 2;
 
diff --git a/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/MakeLDIFTestCase.java b/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/MakeLDIFTestCase.java
index eacb2cf..bd33975 100644
--- a/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/MakeLDIFTestCase.java
+++ b/opendj-ldap-toolkit/src/test/java/com/forgerock/opendj/ldap/tools/MakeLDIFTestCase.java
@@ -104,20 +104,12 @@
             MakeLDIF makeLDIF = new MakeLDIF(outStream, errStream);
             makeLDIF.run(arguments);
 
-            if (expectsResults) {
-                assertThat(out.size()).isGreaterThan(0);
-                assertThat(out.toString("UTF-8")).contains(wrapText(expectedErrOutput, MAX_LINE_WIDTH));
-            } else {
-                if (makeLDIF.isInteractive()) {
-                    assertThat(out.size()).isGreaterThan(0);
-                    assertThat(err.size()).isEqualTo(0);
-                    assertThat(out.toString("UTF-8")).contains(wrapText(expectedErrOutput, MAX_LINE_WIDTH));
-                } else {
-                    assertThat(out.size()).isEqualTo(0);
-                    assertThat(err.size()).isGreaterThan(0);
-                    assertThat(err.toString("UTF-8")).contains(wrapText(expectedErrOutput, MAX_LINE_WIDTH));
-                }
+            assertThat(out.size()).isGreaterThan(0);
+            if (!expectsResults) {
+                assertThat(err.size()).isEqualTo(0);
             }
+            ByteArrayOutputStream std = expectsResults || makeLDIF.isInteractive() ? out : err;
+            assertThat(std.toString("UTF-8")).contains(wrapText(expectedErrOutput, MAX_LINE_WIDTH));
         } finally {
             closeSilently(outStream, errStream);
         }
diff --git a/opendj-server/src/main/java/org/forgerock/opendj/server/setup/cli/SetupCli.java b/opendj-server/src/main/java/org/forgerock/opendj/server/setup/cli/SetupCli.java
index efe9f52..47edaaa 100644
--- a/opendj-server/src/main/java/org/forgerock/opendj/server/setup/cli/SetupCli.java
+++ b/opendj-server/src/main/java/org/forgerock/opendj/server/setup/cli/SetupCli.java
@@ -221,73 +221,43 @@
      */
     private void initializeArguments() throws ArgumentException {
         // Options.
-        acceptLicense = CommonArguments.getAcceptLicense();
-        cli = CommonArguments.getCLI();
-        baseDN = CommonArguments.getBaseDN();
-        addBaseEntry = CommonArguments.getAddBaseEntry();
-        importLDIF = CommonArguments.getImportLDIF();
-        rejectedImportFile = CommonArguments.getRejectedImportLdif();
-        skippedImportFile = CommonArguments.getSkippedImportFile();
-        sampleData = CommonArguments.getSampleData();
-        ldapPort = CommonArguments.getLDAPPort(DEFAULT_LDAP_PORT);
-        adminConnectorPort = CommonArguments.getAdminLDAPPort(DEFAULT_ADMIN_PORT);
-        jmxPort = CommonArguments.getJMXPort(DEFAULT_JMX_PORT);
-        skipPortCheck = CommonArguments.getSkipPortCheck();
-        directoryManagerDN = CommonArguments.getRootDN();
-        directoryManagerPwdString = CommonArguments.getRootDNPwd();
-        directoryManagerPwdFile = CommonArguments.getRootDNPwdFile();
-        enableWindowsService = CommonArguments.getEnableWindowsService();
-        doNotStart = CommonArguments.getDoNotStart();
-        enableStartTLS = CommonArguments.getEnableTLS();
-        ldapsPort = CommonArguments.getLDAPSPort(DEFAULT_LDAPS_PORT);
-        generateSelfSignedCertificate = CommonArguments.getGenerateSelfSigned();
-        hostName = CommonArguments.getHostName(Utils.getDefaultHostName());
-        usePkcs11 = CommonArguments.getUsePKCS11Keystore();
-        useJavaKeyStore = CommonArguments.getUseJavaKeyStore();
-        useJCEKS = CommonArguments.getUseJCEKS();
-        usePkcs12 = CommonArguments.getUsePKCS12KeyStore();
-        keyStorePassword = CommonArguments.getKeyStorePassword();
-        keyStorePasswordFile = CommonArguments.getKeyStorePasswordFile();
-        certNickname = CommonArguments.getCertNickName();
+        acceptLicense = addGlobal(CommonArguments.getAcceptLicense());
+        cli = addGlobal(CommonArguments.getCLI());
+        baseDN = addGlobal(CommonArguments.getBaseDN());
+        addBaseEntry = addGlobal(CommonArguments.getAddBaseEntry());
+        importLDIF = addGlobal(CommonArguments.getImportLDIF());
+        rejectedImportFile = addGlobal(CommonArguments.getRejectedImportLdif());
+        skippedImportFile = addGlobal(CommonArguments.getSkippedImportFile());
+        sampleData = addGlobal(CommonArguments.getSampleData());
+        ldapPort = addGlobal(CommonArguments.getLDAPPort(DEFAULT_LDAP_PORT));
+        ldapsPort = addGlobal(CommonArguments.getLDAPSPort(DEFAULT_LDAPS_PORT));
+        adminConnectorPort = addGlobal(CommonArguments.getAdminLDAPPort(DEFAULT_ADMIN_PORT));
+        jmxPort = addGlobal(CommonArguments.getJMXPort(DEFAULT_JMX_PORT));
+        skipPortCheck = addGlobal(CommonArguments.getSkipPortCheck());
+        directoryManagerDN = addGlobal(CommonArguments.getRootDN());
+        directoryManagerPwdString = addGlobal(CommonArguments.getRootDNPwd());
+        directoryManagerPwdFile = addGlobal(CommonArguments.getRootDNPwdFile());
+        enableWindowsService = addGlobal(CommonArguments.getEnableWindowsService());
+        doNotStart = addGlobal(CommonArguments.getDoNotStart());
+        enableStartTLS = addGlobal(CommonArguments.getEnableTLS());
+        generateSelfSignedCertificate = addGlobal(CommonArguments.getGenerateSelfSigned());
+        hostName = addGlobal(CommonArguments.getHostName(Utils.getDefaultHostName()));
+        usePkcs11 = addGlobal(CommonArguments.getUsePKCS11Keystore());
+        useJavaKeyStore = addGlobal(CommonArguments.getUseJavaKeyStore());
+        useJCEKS = addGlobal(CommonArguments.getUseJCEKS());
+        usePkcs12 = addGlobal(CommonArguments.getUsePKCS12KeyStore());
+        keyStorePassword = addGlobal(CommonArguments.getKeyStorePassword());
+        keyStorePasswordFile = addGlobal(CommonArguments.getKeyStorePasswordFile());
+        certNickname = addGlobal(CommonArguments.getCertNickName());
         connectTimeout = CommonArguments.getConnectTimeOut(DEFAULT_LDAP_CONNECT_TIMEOUT);
 
         // Utility Input Output Options.
-        noPrompt = CommonArguments.getNoPrompt();
-        quietMode = CommonArguments.getQuiet();
-        verbose = CommonArguments.getVerbose();
-        propertiesFile = CommonArguments.getPropertiesFile();
-        noPropertiesFile = CommonArguments.getNoPropertiesFile();
-        showUsage = CommonArguments.getShowUsage();
-
-        // Register global arguments.
-        argParser.addGlobalArgument(cli);
-        argParser.addGlobalArgument(baseDN);
-        argParser.addGlobalArgument(addBaseEntry);
-        argParser.addGlobalArgument(importLDIF);
-        argParser.addGlobalArgument(rejectedImportFile);
-        argParser.addGlobalArgument(skippedImportFile);
-        argParser.addGlobalArgument(sampleData);
-        argParser.addGlobalArgument(ldapPort);
-        argParser.addGlobalArgument(adminConnectorPort);
-        argParser.addGlobalArgument(jmxPort);
-        argParser.addGlobalArgument(skipPortCheck);
-        argParser.addGlobalArgument(directoryManagerDN);
-        argParser.addGlobalArgument(directoryManagerPwdString);
-        argParser.addGlobalArgument(directoryManagerPwdFile);
-        argParser.addGlobalArgument(enableWindowsService);
-        argParser.addGlobalArgument(doNotStart);
-        argParser.addGlobalArgument(enableStartTLS);
-        argParser.addGlobalArgument(ldapsPort);
-        argParser.addGlobalArgument(generateSelfSignedCertificate);
-        argParser.addGlobalArgument(hostName);
-        argParser.addGlobalArgument(usePkcs11);
-        argParser.addGlobalArgument(useJavaKeyStore);
-        argParser.addGlobalArgument(useJCEKS);
-        argParser.addGlobalArgument(usePkcs12);
-        argParser.addGlobalArgument(keyStorePassword);
-        argParser.addGlobalArgument(keyStorePasswordFile);
-        argParser.addGlobalArgument(certNickname);
-        argParser.addGlobalArgument(connectTimeout);
+        noPrompt = addGlobal(CommonArguments.getNoPrompt());
+        quietMode = addGlobal(CommonArguments.getQuiet());
+        verbose = addGlobal(CommonArguments.getVerbose());
+        propertiesFile = addGlobal(CommonArguments.getPropertiesFile());
+        noPropertiesFile = addGlobal(CommonArguments.getNoPropertiesFile());
+        showUsage = addGlobal(CommonArguments.getShowUsage());
 
         //Sub-commands && their arguments
         final ArrayList<SubCommand> subCommandList = new ArrayList<SubCommand>(2);
@@ -314,6 +284,11 @@
         argParser.addArgument(acceptLicense);
     }
 
+    private <A extends Argument> A addGlobal(A arg) throws ArgumentException {
+        argParser.addGlobalArgument(arg);
+        return arg;
+    }
+
     /** {@inheritDoc} */
     @Override
     public boolean isInteractive() {
@@ -375,25 +350,13 @@
             final Set<Integer> ports = new HashSet<Integer>();
             ports.add(ldapPort.getIntValue());
 
-            if (ports.contains(adminConnectorPort.getIntValue())) {
-                errorMessages.add(ERR_PORT_ALREADY_SPECIFIED.get(adminConnectorPort.getIntValue()));
-            } else {
-                ports.add(adminConnectorPort.getIntValue());
-            }
+            checkPortArgument(adminConnectorPort, ports, errorMessages);
 
             if (jmxPort.isPresent()) {
-                if (ports.contains(jmxPort.getIntValue())) {
-                    errorMessages.add(ERR_PORT_ALREADY_SPECIFIED.get(jmxPort.getIntValue()));
-                } else {
-                    ports.add(jmxPort.getIntValue());
-                }
+                checkPortArgument(jmxPort, ports, errorMessages);
             }
             if (ldapsPort.isPresent()) {
-                if (ports.contains(ldapsPort.getIntValue())) {
-                    errorMessages.add(ERR_PORT_ALREADY_SPECIFIED.get(ldapsPort.getIntValue()));
-                } else {
-                    ports.add(ldapsPort.getIntValue());
-                }
+                checkPortArgument(ldapsPort, ports, errorMessages);
             }
         } catch (ArgumentException ae) {
             LOG.error(LocalizableMessage.raw("Unexpected error. "
@@ -401,6 +364,15 @@
         }
     }
 
+    private void checkPortArgument(IntegerArgument portArg, final Set<Integer> ports,
+            final Collection<LocalizableMessage> errorMessages) throws ArgumentException {
+        if (ports.contains(portArg.getIntValue())) {
+            errorMessages.add(ERR_PORT_ALREADY_SPECIFIED.get(portArg.getIntValue()));
+        } else {
+            ports.add(portArg.getIntValue());
+        }
+    }
+
     /**
      * Checks that there are no conflicts with the import data arguments.
      *
diff --git a/opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/Model.java b/opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/Model.java
index e0578e9..b59bbc4 100644
--- a/opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/Model.java
+++ b/opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/Model.java
@@ -91,7 +91,7 @@
      * @return {@code true} if this configuration has a license.
      */
     public boolean hasLicense() {
-        return (this.license != null && !license.isEmpty());
+        return license != null && !license.isEmpty();
     }
 
     /**
@@ -100,7 +100,7 @@
      * @return {@code true} if this configuration is stand alone data store.
      */
     boolean isStandAlone() {
-        return (type == Type.STANDALONE);
+        return type == Type.STANDALONE;
     }
 
     /**
@@ -109,7 +109,7 @@
      * @return {@code true} if this configuration is a first server in a replication topology.
      */
     boolean isFirstInTopology() {
-        return (type == Type.FIRST_IN_TOPOLOGY);
+        return type == Type.FIRST_IN_TOPOLOGY;
     }
 
     /**
@@ -118,7 +118,7 @@
      * @return {@code true} if this configuration is part of a replication topology.
      */
     boolean isPartOfReplicationTopology() {
-        return (type == Type.IN_EXISTING_TOPOLOGY);
+        return type == Type.IN_EXISTING_TOPOLOGY;
     }
 
     /**
@@ -128,7 +128,8 @@
      * @return {@code true} if this configuration has a certificate linked to it.
      */
     boolean isSecure() {
-        return (this.getListenerSettings() != null && this.getListenerSettings().getCertificate() != null);
+        return getListenerSettings() != null
+            && getListenerSettings().getCertificate() != null;
     }
 
     /**
@@ -386,18 +387,18 @@
                 }
             }
         }
-        if (getListenerSettings() == null) {
+        final ListenerSettings settings = getListenerSettings();
+        final DataConfiguration dataConf = getDataConfiguration();
+        if (settings == null) {
             throw new ConfigException(LocalizableMessage.raw("Invalid settings"));
         }
-        if (getDataConfiguration() == null) {
+        if (dataConf == null) {
             throw new ConfigException(LocalizableMessage.raw("Invalid data configuration"));
         }
-        if (getDataConfiguration().isImportLDIF()) {
-            if (getDataConfiguration().getLdifImportDataPath() == null) {
-                throw new ConfigException(LocalizableMessage.raw("Invalid import ldif file."));
-            }
+        if (dataConf.isImportLDIF() && dataConf.getLdifImportDataPath() == null) {
+            throw new ConfigException(LocalizableMessage.raw("Invalid import ldif file."));
         }
-        if (getListenerSettings().getPasswordFile() == null && getListenerSettings().getPassword() == null) {
+        if (settings.getPasswordFile() == null && settings.getPassword() == null) {
             throw new ConfigException(LocalizableMessage.raw("A password must be set for the root DN."));
         }
     }
diff --git a/opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/RuntimeOptions.java b/opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/RuntimeOptions.java
index ad7b531..0fbd88d 100644
--- a/opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/RuntimeOptions.java
+++ b/opendj-server/src/main/java/org/forgerock/opendj/server/setup/model/RuntimeOptions.java
@@ -109,29 +109,24 @@
      */
     @Override()
     public boolean equals(Object o) {
-        boolean equals = o == this;
-        if (!equals) {
-            equals = o instanceof RuntimeOptions;
-            if (equals) {
-                equals = initialMemory == ((RuntimeOptions) o).initialMemory;
-            }
-            if (equals) {
-                equals = maximumMemory == ((RuntimeOptions) o).maximumMemory;
-            }
-            if (equals) {
-                equals = additionalArguments.length == ((RuntimeOptions) o).additionalArguments.length;
-            }
-            if (equals) {
-                String[] args = ((RuntimeOptions) o).additionalArguments;
-                for (int i = 0; i < args.length; i++) {
-                    if (!args[i].equals(additionalArguments[i])) {
-                        equals = false;
-                        break;
-                    }
+        if (o == this) {
+            return true;
+        }
+        if (!(o instanceof RuntimeOptions)) {
+            return false;
+        }
+        final RuntimeOptions other = (RuntimeOptions) o;
+        if (initialMemory == other.initialMemory
+                && maximumMemory == other.maximumMemory
+                && additionalArguments.length == other.additionalArguments.length) {
+            final String[] args = other.additionalArguments;
+            for (int i = 0; i < args.length; i++) {
+                if (!args[i].equals(additionalArguments[i])) {
+                    return false;
                 }
             }
         }
-        return equals;
+        return true;
     }
 
     /**
diff --git a/opendj-server/src/test/java/org/forgerock/opendj/server/setup/cli/SetupCliTestCase.java b/opendj-server/src/test/java/org/forgerock/opendj/server/setup/cli/SetupCliTestCase.java
index 9af3166..9455abb 100644
--- a/opendj-server/src/test/java/org/forgerock/opendj/server/setup/cli/SetupCliTestCase.java
+++ b/opendj-server/src/test/java/org/forgerock/opendj/server/setup/cli/SetupCliTestCase.java
@@ -48,10 +48,9 @@
  */
 public class SetupCliTestCase extends AbstractSetupCliTestCase {
 
-    // @formatter:off
     @DataProvider(name = "validArguments")
     Object[][] createValidArguments() throws Exception {
-        Object[][] data = new Object[][] {
+        return  new Object[][] {
             { args("--help"),
                 expectedErrOutput(INFO_SETUP_DESCRIPTION.get()) },
             { args("--cli", "create-directory-server", "--doNotStart", "--ldapPort", "1389",
@@ -62,24 +61,22 @@
                     "-D", "cn=Directory Manager", "-w", "password", "-b", "dc=example,dc=com",
                     "-a", "--ldapsPort", "1636", "--generateSelfSignedCertificate"), null },
         };
-        return data;
     }
 
     @DataProvider(name = "invalidArguments")
     Object[][] createInValidArguments() throws Exception {
-        Object[][] data = new Object[][] {
+        return new Object[][] {
             { args("-c"),
                 expectedErrOutput(
                         ERR_ERROR_PARSING_ARGS.get(ERR_SUBCMDPARSER_NO_GLOBAL_ARGUMENT_FOR_SHORT_ID.get("c"))) },
             { args("-N"), expectedErrOutput(ERR_ERROR_PARSING_ARGS.get(
                     ERR_ARGPARSER_NO_VALUE_FOR_ARGUMENT_WITH_SHORT_ID.get("N"))) },
         };
-        return data;
     }
 
     @DataProvider(name = "validPorts")
     Object[][] createValidPorts() throws Exception {
-        Object[][] data = new Object[][] {
+        return new Object[][] {
             { args("--cli", "--doNotStart", "--ldapPort", "1389", "--adminConnectorPort", "4444",
                     "-D", "cn=Directory Manager", "-w", "password", "-b", "dc=example,dc=com",
                     "-a"), null },
@@ -90,12 +87,11 @@
                     "-D", "cn=Directory Manager", "-w", "password", "-b", "dc=example,dc=com",
                     "-a", "--jmxPort", "1689"), null },
         };
-        return data;
     }
 
     @DataProvider(name = "invalidPorts")
     Object[][] createInValidPorts() throws Exception {
-        Object[][] data = new Object[][] {
+        return new Object[][] {
             { args("--cli", "--doNotStart", "--ldapPort", "1389", "--adminConnectorPort", "4444",
                     "-D", "cn=Directory Manager", "-w", "password", "-b", "dc=example,dc=com",
                     "-a", "--jmxPort", "1389"),
@@ -119,9 +115,7 @@
                         ERR_ARGPARSER_VALUE_UNACCEPTABLE_FOR_LONG_ID.get(-1, "ldapPort",
                                 ERR_INTARG_VALUE_BELOW_LOWER_BOUND.get("ldapPort", -1, 1)))) },
         };
-        return data;
     }
-    // @formatter:on
 
     @Test(dataProvider = "validArguments")
     public void testRunValidArguments(String[] arguments, LocalizableMessage expectedErrOutput) throws Exception {
@@ -166,20 +160,15 @@
                 assertThat(resultCode).isEqualTo(ReturnCode.SUCCESS.get());
             } else {
                 assertThat(resultCode).isNotEqualTo(ReturnCode.SUCCESS.get());
-                String errMsg = null;
-                String expectedMsg = getUnWrappedMessage(expectedErrOutput.toString());
                 /**
                  * If an application is interactive, all messages should be redirect to the stdout. (info, warnings,
                  * errors). Otherwise, standard messages should be displayed in the stdout(info) and errors to the
                  * stderr (warnings, errors).
                  */
-                if (setup.isInteractive()) {
-                    assertThat(out.size()).isGreaterThan(0);
-                    errMsg = getUnWrappedMessage(out.toString("UTF-8"));
-                } else {
-                    assertThat(err.size()).isGreaterThan(0);
-                    errMsg = getUnWrappedMessage(err.toString("UTF-8"));
-                }
+                ByteArrayOutputStream std = setup.isInteractive() ? out : err;
+                assertThat(std.size()).isGreaterThan(0);
+                String errMsg = getUnWrappedMessage(std.toString("UTF-8"));
+                String expectedMsg = getUnWrappedMessage(expectedErrOutput.toString());
                 assertTrue(errMsg.contains(expectedMsg), errMsg + "\n >---< \n" + expectedMsg);
             }
         } finally {

--
Gitblit v1.10.0