Locale-related code -- are there corner cases I didn't see?

Question

This is a utility class I have in one of my projects and I seek feedback on it. I have read the javadoc for Locale quite a few times while developing this code. I think I have it right, but maybe there are holes in it...

Reviews welcome; also, if there are parts of the code you don't understand due to lack of comments, I'd like to hear about that as well.

Link to the original source: here; test file for this class: here (not pasted in this post).

Note: this library is meant to work with Java 6+; I know of the Locale.Builder class, however this class only appeared in Java 7.

/**
 * Utility methods for {@link Locale} management
 *
 * <p>This class provides two methods:</p>
 *
 * <ul>
 *     <li>{@link #parseLocale(String)} parses a string and builds a {@link
 *     Locale} object (strangely enough, there is no such method in the JDK!);
 *     </li>
 *     <li>{@link #getApplicable(Locale)} returns an ordered list of locales
 *     "applicable" to the given locale.</li>
 * </ul>
 *
 * <p>The {@link #getApplicable(Locale)} method emulates what the JDK's {@link
 * ResourceBundle} does when you look up a message in a locale; it returns an
 * ordered list from the most specific to the more general. For instance, given
 * the locale {@code "ja_JP_JP"}, it will generate the following list:</p>
 *
 * <ul>
 *     <li>{@code "ja_JP_JP"},</li>
 *     <li>{@code "ja_JP"},</li>
 *     <li>{@code "ja"},</li>
 *     <li>{@code ""} (the root locale, {@link Locale#ROOT}).</li>
 * </ul>
 */
public final class LocaleUtils
{
    private static final Pattern UNDERSCORE = Pattern.compile("_");

    private LocaleUtils()
    {
    }

    /**
     * Parse a string input as an argument and return a locale object
     *
     * <p>Three things to note:</p>
     *
     * <ul>
     *     <li>it is NOT checked whether the extracted language or country codes
     *     are actually registered to the ISO;</li>
     *     <li>all input strings with more than two underscores are deemed
     *     illegal;</li>
     *     <li>if the first component (the language) is empty, {@link
     *     Locale#ROOT} is returned.</li>
     * </ul>
     *
     * @see Locale
     *
     * @param input the input string
     * @throws NullPointerException input is null
     * @throws IllegalArgumentException input is malformed (see above)
     * @return a {@link Locale}
     */
    public static Locale parseLocale(final String input)
    {
        if (input == null)
            throw new NullPointerException("input cannot be null");

        if (input.isEmpty())
            return Locale.ROOT;

        /*
         * NOTE NOTE NOTE: in order for .split() to behave in a sane manner, we
         * MUST use the "multi-argument" version of .split() with a negative
         * argument. The no-argument version (this also stands for String's
         * .split()) will remove all empty strings from the end of the resulting
         * array up to the first non empty element.
         *
         * I don't know who designed this API, but he should either be given
         * the boot or killed[1].
         *
         * [1] choose the better option; hint: choose option 2
         */
        final String[] elements = UNDERSCORE.split(input, -1);
        final int len = elements.length;

        if (len > 3)
            throw new IllegalArgumentException("malformed input " + input);
        if (elements[0].isEmpty())
            return Locale.ROOT;

        switch (len) {
            case 1:
                return new Locale(elements[0]);
            case 2:
                return new Locale(elements[0], elements[1]);
            case 3:
                return new Locale(elements[0], elements[1], elements[2]);
            default:
                throw new IllegalStateException("How did I get there??");
        }
    }

    /**
     * Get a "decrementing" list of candidate locales for a given locale
     *
     * <p>The order of locale returned is from the more specific to the less
     * specific (the latter being {@link Locale#ROOT}).</p>
     *
     * @param target the locale
     * @return the list of applicable locales
     */
    public static Collection<Locale> getApplicable(final Locale target)
    {
        final String language = target.getLanguage();
        final String country = target.getCountry();
        final String variant = target.getVariant();

        final List<Locale> ret = new ArrayList<Locale>();
        ret.add(target);

        Locale locale;

        if (!variant.isEmpty()) {
            locale = new Locale(language, country);
            if (!locale.equals(Locale.ROOT))
                ret.add(locale);
        }

        if (!country.isEmpty()) {
            locale = new Locale(language);
            if (!locale.equals(Locale.ROOT))
                ret.add(locale);
        }

        if (!language.isEmpty())
            ret.add(Locale.ROOT);

        return ret;
    }
}

Answer 1

In general, your code is well structured, and well documented.

As for the -1 requirement for split, Java has done the right thing by keeping the behaviour consistent with the Perl RegEx library .... so blame Perl (and perhaps Perl can blame someone else).

My more significant issue is that, based on the fact that you have written this utility class, I expect that you are calling these methods 'often'. Since Local instances can be slow to create from scratch, I imagine that you have a performance problem too. Creating many Locale instances is not a great idea when they are thread-safe, and can be reused easily between threads/invocations.

My recommendation to you would be to use a java.util.concurrent.ConcurrentHashMap to store the Locales you create, and to do a lookup from the Map first, and only create the Locale if you need to.

Something like:

// rename parseLocale(String input) and make it private.....
private static final internalParseLocale(String ...) {
}

private static final ConcurrentHashMap<String, Locale> localemap = new ConcurrentHashMap<> ();

// new public method
public static final Locale parseLocale(final String input) {
    final Locale gotit = localemap.get(input);
    if (gotit != null) {
        return gotit;
    }
    final Locale created = internalParseLocale(input);
    final Locale wasthere = localemap.putIfAbsent(input, created);
    if (wasthere != null) {
        // there was a race condition, and we lost!
        return wasthere;
    }
    return created;
}

Then, in your getApplicable() method you should:

• avoid using new Locale(...) but instead use parseLocale(.....)
• reuse the locales there too