Code cleanup.

Jean-Noel Rouvignac

15.49.2015 0565fb356eb92ac014ff0e3851f88b0967ffaae1

Code cleanup.

Argument.java:
In getDoubleValue(), fixed erroneous error messages referring to int.

ArgumentParser.java:
I find the way versionArgument is handled is a bit strange. It should be added to the general argument and custom null-checking to display the version code should be removed.

 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();
    }
}

 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 + ")";
    }
}

 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) {

 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 \