SHOUTY_SNAKE_CASED NUMBERS

Posted on

Problem

WE_ALL_LOVE_SHOUTY_SNAKE_CASE_SO_I_WROTE_A_PROGRAM_FOR_IT!!1!

Alright, enough of this. This is a from this question. The pattern is pretty simple, it takes a String input, verifies if it is numerical, and if so, converts it to SHOUTY_SNAKE_CASE.

Example : “1231” -> “ONE_TWO_THREE_ONE”

It is fairly simple but it is also the first time I used the Java 8’s streams, so I wanted an opinion on my (simple) use of it.

private static final String[] words = 
        new String[]{"zero","one","two","three","four","five","six","seven","eight","nine"};

public static String convertNumbersToWords(final String input) {

    if(input == null || !input.matches("^\d*$")){
        throw new IllegalArgumentException("input cannot be null or non-numerical.");
    }

    return input.chars()
             .mapToObj(c -> getWordFromCharCode(c).toUpperCase())
             .collect(Collectors.joining("_"));
}

private static String getWordFromCharCode(int code){
    return words[code - 48];
}

Solution

Since you need an enumerated set of LOUD_WORDS that aligns with numbers, preferably as some form of numeric indices… an enum instead of String[] seems to be the perfect fit:

enum Word {
    ZERO, ONE, TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE;
}

(I think you can figure out the derivation.)

edit: Just one note about using Word.values() as pointed out in @Simon‘s comments: it creates a new array each time and this may not be desirable, performance-wise.

Since you’re also experimenting with some of the Java 8 features, you may want to consider using an Optional too, to eliminate the if-validation:

private static final String NUMBERS_ONLY = "^\d+$";

// Usage
Optional.ofNullable(input)
        .filter(v -> v != null && v.matches(NUMBERS_ONLY))
        .orElseThrow(IllegalArgumentException::new)
        .chars()
        // ...

If you prefer to throw NullPointerException for null values, you can stick with Optional.of(T).

Why are you storing them as lowercase words only to call toUpperCase on them? Store them in the form you intend to use them and save yourself a function call on each word. It’s also more understandable to have the strings stored the same way
they’ll be printed.

I would propose replacing the kinda low-level code - 48 with Character.getNumericValue.

Another consideration might be to use guava’s CharMatcher.DIGIT.matchesAllOf instead of the regex for validating the input string, though I realize that using libraries is often besides the point for CR.stackexchange.
Note that this approach would pass you digits with numeric values bigger than 9 as well (i.e. roman numeral 50) and you’d have to think of a way to support them. (Split to single digits?)

Naming and method decomposition

Let’s take a look at this method signature:

public static String convertNumbersToWords(final String input) {

The method name suggests converting numbers… And it works with inputs like “1234”. It’s a bit unnatural that the input is a string, as opposed to a numeric type, and “1234” is one number, not plural numbers.

In addition, the method does two things:

  • validate the input consists of digits only
  • convert the digits to SHOUT_CASE

It would be better to decompose this to two methods, for example:

private static String convertValidatedDigitsToWords(final String digits) {
    return digits.chars()
            .mapToObj(ShoutySnake::getWordFromCharCode)
            .collect(Collectors.joining("_"));
}

public static String convertDigitsToWords(final String input) {
    if (input == null || !input.matches("\d*")) {
        throw new IllegalArgumentException("input cannot be null or non-numerical.");
    }
    return convertValidatedDigitsToWords(input);
}

… and perhaps add an overloading for numeric input:

public static String convertDigitsToWords(final int number) {
    if (number < 0) {
        throw new IllegalArgumentException("input must be non-negative");
    }
    return convertValidatedDigitsToWords(Integer.toString(number));
}

A small related tip: String.matches implies ^ and $, no need to include that in the pattern.

Optimizing for repeated calls

If you expect the method to be called repeatedly,
then it would be good to compile the regular expression.

private static final Pattern pattern = Pattern.compile("\d*");

public static String convertDigitsToWords(final String input) {
    if (!pattern.matcher(input).matches()) {
        throw new IllegalArgumentException("input must be non-null and non-negative.");
    }
    return convertValidatedDigitsToWords(input);
}

Magic number 48

In this code, 48 is a magic number:

private static String getWordFromCharCode(int code){
    return words[code - 48];
}

You can easily make it less magic by referring to '0' instead:

private static String getWordFromCharCode(int code) {
    return words[code - '0'];
}

The name convertNumbersToWords() does not clearly reflect what the method is doing. You can either do this using the java convention like convertNumbersToShoutySnakeCaseWords() or in a way it matches the purpose of the application like CONVERT_NUMBERS_TO_SHOUTY_SNAKE_CASE_WORDS()

Leave a Reply

Your email address will not be published. Required fields are marked *