From 0565fb356eb92ac014ff0e3851f88b0967ffaae1 Mon Sep 17 00:00:00 2001
From: Jean-Noel Rouvignac <jean-noel.rouvignac@forgerock.com>
Date: Thu, 15 Jan 2015 10:49:31 +0000
Subject: [PATCH] Code cleanup.

---
 opendj-cli/src/main/java/com/forgerock/opendj/cli/Argument.java       |   84 ++++++++++----------
 opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentGroup.java  |   15 ++-
 opendj-cli/src/main/resources/com/forgerock/opendj/cli/cli.properties |    6 +
 opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentParser.java |  120 +++++++----------------------
 4 files changed, 88 insertions(+), 137 deletions(-)

diff --git a/opendj-cli/src/main/java/com/forgerock/opendj/cli/Argument.java b/opendj-cli/src/main/java/com/forgerock/opendj/cli/Argument.java
index d3316d0..de014a4 100644
--- a/opendj-cli/src/main/java/com/forgerock/opendj/cli/Argument.java
+++ b/opendj-cli/src/main/java/com/forgerock/opendj/cli/Argument.java
@@ -22,12 +22,12 @@
  *
  *
  *      Copyright 2006-2008 Sun Microsystems, Inc.
- *      Portions copyright 2014 ForgeRock AS
+ *      Portions copyright 2014-2015 ForgeRock AS
  */
 package com.forgerock.opendj.cli;
 
 import static com.forgerock.opendj.cli.CliMessages.*;
-import static com.forgerock.opendj.util.StaticUtils.toLowerCase;
+import static com.forgerock.opendj.util.StaticUtils.*;
 
 import java.util.Iterator;
 import java.util.LinkedList;
@@ -154,13 +154,11 @@
         this.isValueSetByProperty = false;
 
         if (shortIdentifier == null && longIdentifier == null) {
-            final LocalizableMessage message = ERR_ARG_NO_IDENTIFIER.get(name);
-            throw new ArgumentException(message);
+            throw new ArgumentException(ERR_ARG_NO_IDENTIFIER.get(name));
         }
 
         if (needsValue && valuePlaceholder == null) {
-            final LocalizableMessage message = ERR_ARG_NO_VALUE_PLACEHOLDER.get(name);
-            throw new ArgumentException(message);
+            throw new ArgumentException(ERR_ARG_NO_VALUE_PLACEHOLDER.get(name));
         }
 
         values = new LinkedList<String>();
@@ -213,8 +211,7 @@
                 || "off".equals(valueString) || "0".equals(valueString)) {
             return false;
         } else {
-            throw new ArgumentException(
-                    ERR_ARG_CANNOT_DECODE_AS_BOOLEAN.get(valueString, name));
+            throw new ArgumentException(ERR_ARG_CANNOT_DECODE_AS_BOOLEAN.get(valueString, name));
         }
     }
 
@@ -251,26 +248,19 @@
      */
     public double getDoubleValue() throws ArgumentException {
         if (values.isEmpty()) {
-            final LocalizableMessage message = ERR_ARG_NO_INT_VALUE.get(name);
-            throw new ArgumentException(message);
+            throw new ArgumentException(ERR_ARG_NO_INT_VALUE.get(name));
         }
 
         final Iterator<String> iterator = values.iterator();
         final String valueString = iterator.next();
-
-        double intValue;
-        try {
-            intValue = Double.parseDouble(valueString);
-        } catch (final Exception e) {
-            final LocalizableMessage message = ERR_ARG_CANNOT_DECODE_AS_INT.get(valueString, name);
-            throw new ArgumentException(message, e);
+        if (iterator.hasNext()) {
+            throw new ArgumentException(ERR_ARG_INT_MULTIPLE_VALUES.get(name));
         }
 
-        if (iterator.hasNext()) {
-            final LocalizableMessage message = ERR_ARG_INT_MULTIPLE_VALUES.get(name);
-            throw new ArgumentException(message);
-        } else {
-            return intValue;
+        try {
+            return Double.parseDouble(valueString);
+        } catch (final Exception e) {
+            throw new ArgumentException(ERR_ARG_CANNOT_DECODE_AS_INT.get(valueString, name), e);
         }
     }
 
@@ -292,9 +282,7 @@
             try {
                 intList.add(Double.valueOf(valueString));
             } catch (final Exception e) {
-                final LocalizableMessage message =
-                        ERR_ARG_CANNOT_DECODE_AS_INT.get(valueString, name);
-                throw new ArgumentException(message, e);
+                throw new ArgumentException(ERR_ARG_CANNOT_DECODE_AS_INT.get(valueString, name), e);
             }
         }
 
@@ -311,26 +299,19 @@
      */
     public int getIntValue() throws ArgumentException {
         if (values.isEmpty()) {
-            final LocalizableMessage message = ERR_ARG_NO_INT_VALUE.get(name);
-            throw new ArgumentException(message);
+            throw new ArgumentException(ERR_ARG_NO_INT_VALUE.get(name));
         }
 
         final Iterator<String> iterator = values.iterator();
         final String valueString = iterator.next();
-
-        int intValue;
-        try {
-            intValue = Integer.parseInt(valueString);
-        } catch (final Exception e) {
-            final LocalizableMessage message = ERR_ARG_CANNOT_DECODE_AS_INT.get(valueString, name);
-            throw new ArgumentException(message, e);
+        if (iterator.hasNext()) {
+            throw new ArgumentException(ERR_ARG_INT_MULTIPLE_VALUES.get(name));
         }
 
-        if (iterator.hasNext()) {
-            final LocalizableMessage message = ERR_ARG_INT_MULTIPLE_VALUES.get(name);
-            throw new ArgumentException(message);
-        } else {
-            return intValue;
+        try {
+            return Integer.parseInt(valueString);
+        } catch (final Exception e) {
+            throw new ArgumentException(ERR_ARG_CANNOT_DECODE_AS_INT.get(valueString, name), e);
         }
     }
 
@@ -352,9 +333,7 @@
             try {
                 intList.add(Integer.valueOf(valueString));
             } catch (final Exception e) {
-                final LocalizableMessage message =
-                        ERR_ARG_CANNOT_DECODE_AS_INT.get(valueString, name);
-                throw new ArgumentException(message, e);
+                throw new ArgumentException(ERR_ARG_CANNOT_DECODE_AS_INT.get(valueString, name), e);
             }
         }
 
@@ -648,4 +627,25 @@
      */
     public abstract boolean valueIsAcceptable(String valueString,
             LocalizableMessageBuilder invalidReason);
+
+    /** {@inheritDoc} */
+    @Override
+    public String toString() {
+        final StringBuilder sb = new StringBuilder();
+        sb.append(getClass().getSimpleName());
+        sb.append("(");
+        if (longIdentifier != null) {
+            sb.append("longID=");
+            sb.append(longIdentifier);
+        }
+        if (shortIdentifier != null) {
+            if (longIdentifier != null) {
+                sb.append(", ");
+            }
+            sb.append("shortID=");
+            sb.append(shortIdentifier);
+        }
+        sb.append(")");
+        return sb.toString();
+    }
 }
diff --git a/opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentGroup.java b/opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentGroup.java
index 3c33bb3..d6f24b5 100644
--- a/opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentGroup.java
+++ b/opendj-cli/src/main/java/com/forgerock/opendj/cli/ArgumentGroup.java
@@ -22,7 +22,7 @@
  *
  *
  *      Copyright 2008 Sun Microsystems, Inc.
- *      Portions copyright 2012-2014 ForgeRock AS.
+ *      Portions copyright 2012-2015 ForgeRock AS.
  */
 package com.forgerock.opendj.cli;
 
@@ -41,10 +41,8 @@
 
     /** Description for this group of arguments. */
     private LocalizableMessage description;
-
     /** List of arguments belonging to this group. */
     private List<Argument> args;
-
     /** Governs groups position within usage statement. */
     private final Integer priority;
 
@@ -64,6 +62,7 @@
     }
 
     /** {@inheritDoc} */
+    @Override
     public int compareTo(final ArgumentGroup o) {
         // Groups with higher priority numbers appear before
         // those with lower priority in the usage output
@@ -78,7 +77,6 @@
      * @return boolean where true indicates the add was successful
      */
     public boolean addArgument(final Argument arg) {
-        boolean success = false;
         if (arg != null) {
             final Character newShort = arg.getShortIdentifier();
             final String newLong = arg.getLongIdentifier();
@@ -94,9 +92,9 @@
                 }
             }
 
-            success = this.args.add(arg);
+            return this.args.add(arg);
         }
-        return success;
+        return false;
     }
 
     /**
@@ -161,4 +159,9 @@
         this.description = description;
     }
 
+    /** {@inheritDoc} */
+    @Override
+    public String toString() {
+        return getClass().getSimpleName() + "(description=" + description + ")";
+    }
 }
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 2f33aaf..30759fb 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
@@ -22,12 +22,12 @@
  *
  *
  *      Copyright 2006-2010 Sun Microsystems, Inc.
- *      Portions copyright 2012-2014 ForgeRock AS.
+ *      Portions copyright 2012-2015 ForgeRock AS.
  */
 package com.forgerock.opendj.cli;
 
-import static com.forgerock.opendj.cli.CliMessages.*;
 import static com.forgerock.opendj.cli.ArgumentConstants.*;
+import static com.forgerock.opendj.cli.CliMessages.*;
 import static com.forgerock.opendj.cli.Utils.*;
 import static com.forgerock.opendj.util.StaticUtils.*;
 
@@ -66,25 +66,14 @@
      * directory.
      */
     public static final String DEFAULT_OPENDJ_CONFIG_DIR = ".opendj";
-
-    /**
-     * The default properties file name.
-     */
+    /** The default properties file name. */
     public static final String DEFAULT_OPENDJ_PROPERTIES_FILE_NAME = "tools";
-
-    /**
-     * The default properties file extension.
-     */
+    /** The default properties file extension. */
     public static final String DEFAULT_OPENDJ_PROPERTIES_FILE_EXTENSION = ".properties";
-
-    /**
-     * The name of a command-line script used to launch a tool.
-     */
+    /** The name of a command-line script used to launch a tool. */
     public static final String PROPERTY_SCRIPT_NAME = "com.forgerock.opendj.ldap.tools.scriptName";
 
-    /**
-     * The argument that will be used to indicate the file properties.
-     */
+    /** The argument that will be used to indicate the file properties. */
     private StringArgument filePropertiesPathArgument;
 
     /**
@@ -105,10 +94,8 @@
      */
     private Argument versionArgument;
 
-    /**
-     * The set of unnamed trailing arguments that were provided for this parser.
-     */
-    private final ArrayList<String> trailingArguments;
+    /** The set of unnamed trailing arguments that were provided for this parser. */
+    private final ArrayList<String> trailingArguments = new ArrayList<String>();
 
     /**
      * Indicates whether this parser will allow additional unnamed arguments at
@@ -122,48 +109,29 @@
      */
     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. */
     private boolean versionPresent;
-
     /** The handler to call to print the product version. */
     private VersionHandler versionHandler;
 
-    /**
-     * 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 short ID. */
+    private final HashMap<Character, Argument> shortIDMap = new HashMap<Character, Argument>();
+    /** The set of arguments defined for this parser, referenced by long ID. */
+    private final HashMap<String, Argument> longIDMap = new HashMap<String, Argument>();
+    /** The set of arguments defined for this parser, referenced by argument name. */
+    private final HashMap<String, Argument> argumentMap = new HashMap<String, Argument>();
+    /** The total set of arguments defined for this parser. */
+    private final LinkedList<Argument> argumentList = new LinkedList<Argument>();
 
-    /**
-     * 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.
-     */
-    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. */
-    private final LinkedList<Argument> argumentList;
-
     /** The output stream to which usage information should be printed. */
-    private OutputStream usageOutputStream;
+    private OutputStream usageOutputStream = System.out;
 
     /**
      * The fully-qualified name of the Java class that should be invoked to
@@ -241,22 +209,10 @@
         this.toolDescription = toolDescription;
         this.longArgumentsCaseSensitive = longArgumentsCaseSensitive;
 
-        argumentList = new LinkedList<Argument>();
-        argumentMap = new HashMap<String, Argument>();
-        shortIDMap = new HashMap<Character, Argument>();
-        longIDMap = new HashMap<String, Argument>();
         allowsTrailingArguments = false;
-        usageOrVersionDisplayed = false;
-        versionPresent = false;
         trailingArgsDisplayName = null;
         maxTrailingArguments = 0;
         minTrailingArguments = 0;
-        trailingArguments = new ArrayList<String>();
-        rawArguments = null;
-        usageArgument = null;
-        filePropertiesPathArgument = null;
-        noPropertiesFileArgument = null;
-        usageOutputStream = System.out;
         initGroups();
     }
 
@@ -301,16 +257,6 @@
         this.maxTrailingArguments = maxTrailingArguments;
         this.trailingArgsDisplayName = trailingArgsDisplayName;
 
-        argumentList = new LinkedList<Argument>();
-        argumentMap = new HashMap<String, Argument>();
-        shortIDMap = new HashMap<Character, Argument>();
-        longIDMap = new HashMap<String, Argument>();
-        trailingArguments = new ArrayList<String>();
-        usageOrVersionDisplayed = false;
-        versionPresent = false;
-        rawArguments = null;
-        usageArgument = null;
-        usageOutputStream = System.out;
         initGroups();
     }
 
@@ -341,7 +287,6 @@
      *             has already been defined.
      */
     public void addArgument(final Argument argument, ArgumentGroup group) throws ArgumentException {
-
         final Character shortID = argument.getShortIdentifier();
         if (shortID != null && shortIDMap.containsKey(shortID)) {
             final String conflictingName = shortIDMap.get(shortID).getName();
@@ -349,12 +294,14 @@
                     ERR_ARGPARSER_DUPLICATE_SHORT_ID.get(argument.getName(), shortID, conflictingName));
         }
 
+        // JNR: what is the requirement for the following code?
         if (versionArgument != null
                 && shortID != null
                 && shortID.equals(versionArgument.getShortIdentifier())) {
             // Update the version argument to not display its short identifier.
             try {
                 versionArgument = getVersionArgument(false);
+                // JNR: why not call addGeneralArgument(versionArgument) here?
                 this.generalArgGroup.addArgument(versionArgument);
             } catch (final ArgumentException e) {
                 // ignore
@@ -368,11 +315,8 @@
             }
             if (longIDMap.containsKey(longID)) {
                 final String conflictingName = longIDMap.get(longID).getName();
-
-                final LocalizableMessage message =
-                        ERR_ARGPARSER_DUPLICATE_LONG_ID.get(argument.getName(), argument
-                                .getLongIdentifier(), conflictingName);
-                throw new ArgumentException(message);
+                throw new ArgumentException(ERR_ARGPARSER_DUPLICATE_LONG_ID.get(
+                    argument.getName(), argument.getLongIdentifier(), conflictingName));
             }
         }
 
@@ -665,17 +609,15 @@
      * @return argument group appropriate for <code>argument</code>
      */
     ArgumentGroup getStandardGroup(final Argument argument) {
-        ArgumentGroup group;
         if (isInputOutputArgument(argument)) {
-            group = ioArgGroup;
+            return ioArgGroup;
         } else if (isGeneralArgument(argument)) {
-            group = generalArgGroup;
+            return generalArgGroup;
         } else if (isLdapConnectionArgument(argument)) {
-            group = ldapArgGroup;
+            return ldapArgGroup;
         } else {
-            group = defaultArgGroup;
+            return defaultArgGroup;
         }
-        return group;
     }
 
     /**
@@ -1366,6 +1308,7 @@
 
         try {
             versionArgument = getVersionArgument(true);
+            // JNR: why not call addGeneralArgument(versionArgument) here?
             this.generalArgGroup.addArgument(versionArgument);
         } catch (final ArgumentException e) {
             // ignore
@@ -1373,12 +1316,11 @@
     }
 
     private boolean isGeneralArgument(final Argument arg) {
-        boolean general = false;
         if (arg != null) {
             final String longId = arg.getLongIdentifier();
-            general = OPTION_LONG_HELP.equals(longId) || OPTION_LONG_PRODUCT_VERSION.equals(longId);
+            return OPTION_LONG_HELP.equals(longId) || OPTION_LONG_PRODUCT_VERSION.equals(longId);
         }
-        return general;
+        return false;
     }
 
     private boolean isInputOutputArgument(final Argument arg) {
diff --git a/opendj-cli/src/main/resources/com/forgerock/opendj/cli/cli.properties b/opendj-cli/src/main/resources/com/forgerock/opendj/cli/cli.properties
index 25c66cf..c72644b 100755
--- a/opendj-cli/src/main/resources/com/forgerock/opendj/cli/cli.properties
+++ b/opendj-cli/src/main/resources/com/forgerock/opendj/cli/cli.properties
@@ -37,6 +37,12 @@
  argument cannot be decoded as an integer
 ERR_ARG_INT_MULTIPLE_VALUES=The %s argument has multiple values and \
  therefore cannot be decoded as a single integer value
+ERR_ARG_NO_DOUBLE_VALUE=The %s argument does not have any value that \
+ may be retrieved as a double
+ERR_ARG_CANNOT_DECODE_AS_DOUBLE=The provided value "%s" for the %s \
+ argument cannot be decoded as a double
+ERR_ARG_DOUBLE_MULTIPLE_VALUES=The %s argument has multiple values and \
+ therefore cannot be decoded as a single double value
 ERR_ARG_NO_BOOLEAN_VALUE=The %s argument does not have any value \
  that may be retrieved as a Boolean
 ERR_ARG_CANNOT_DECODE_AS_BOOLEAN=The provided value "%s" for the %s \

--
Gitblit v1.10.0