HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

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

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
didncornerarerelatedseelocalecodetherecases

Problem

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
*
* This class provides two methods:
*
*
* {@link #parseLocale(String)} parses a string and builds a {@link
* Locale} object (strangely enough, there is no such method in the JDK!);
*
* {@link #getApplicable(Locale)} returns an ordered list of locales
* "applicable" to the given locale.
*
*
* 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:
*
*
* {@code "ja_JP_JP"},
* {@code "ja_JP"},
* {@code "ja"},
* {@code ""} (the root locale, {@link Locale#ROOT}).
*
*/
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
*
* Three things to note:
*
*
* it is NOT checked whether the extracted language or country codes
* are actually registered to the ISO;
* all input strings with more than two underscores are deemed
* illegal;
* if the first component (the language) is empty, {@link
* Locale#ROOT} is

Solution

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

Code Snippets

// 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;
}

Context

StackExchange Code Review Q#26877, answer score: 5

Revisions (0)

No revisions yet.