Long-to-string and string-to-long while checking characters within a xlate table

Question

I am attempting to come up with a better method for doing this:

public static String longToPlayerName(long name) {
    int i = 0;
    char[] nameCharacters = new char[12];
    while (name != 0L) {
        long ll = name;
        name /= 37L;
        nameCharacters[11 - i++] = playerNameXlateTable[(int) (ll - (name * 37L))];
    }
    return new String(nameCharacters, 12 - i, i);
}

public static long playerNameToInt64(String s) {
    long l = 0L;
    for (int i = 0; (i < s.length()) && (i < 12); i++) {
        final char c = s.charAt(i);
        l *= 37L;
        if ((c >= 'A') && (c <= 'Z')) {
            l += (1 + c) - 65;
        } else if ((c >= 'a') && (c <= 'z')) {
            l += (1 + c) - 97;
        } else if ((c >= '0') && (c <= '9')) {
            l += (27 + c) - 48;
        }
    }
    while (((l % 37L) == 0L) && (l != 0L)) {
        l /= 37L;
    }
    return l;
}

I can't seem to come up with a better way. Any ideas would be excellent!

Answer 1

Pretty good overall, but a few things:

• Don't hard-code the 37. Instead, use the translation table's size.
• ll is a bad variable name. It's not descriptive at all, and it looks like 11.

• Your loop in longToPlayerName can be simplified:

  final int base = playerNameXlateTable.length;
  while (name != 0L) {
      final long ch = name % base;
      nameCharacters[11 - i++] = playerNameXlateTable[ch];
      name /= base;
  }
  

• Be careful with inlined ++i or i++. They can be easy to overlook, and the human brain (mine anyway) doesn't parse arithmetic expressions that cause mutations very well.

• I think you've overused parenthesis a bit. For example, I would just do:

  if (c >= 'A' && c <= 'Z') {
  

• playerNameToInt64 should be playerNameToLong

• s is a bad parameter name. Call it something like name
  • Other than for loop controls, single letter variables should typically be avoided.
  • It should also probably be final, but it's immutable, so it doesn't really matter in this case.
• Use Character.toLowerCase to eliminate one of the branches in playerNameToInt64.
• It might be better to group your shifts like 1 + (c - 97) instead of how they're grouped. It doesn't matter in the current code, but if (1 + c) - 97 can in theory overflow, but 1 + (c - 97) cannot

• Rather than 97/65/48, use character literals:

  1 + (c - 'a')
  

• The truncation loop is a bit troubling. For a-zA-Z0-9, there never will be any trailing 0 valued base-37 digit. This means that you're worried an invalid character will be provided. But... what if one is in the middle of the string?

• If you made a reverse translation table, you could decouple your code from ASCII
  • no point in doing this though, as it's probably always going to be ASCII (or a superset of ASCII)
• Fold the double comparison in the toLong loop into a Math.min() call and use that
• Assuming this class is dedicated to player name translation and only player name translation (as it should be), playerNameXlateTable could be shortened to translationTable
  • In the context of a single-responsibility class, it's obvious what the table is translating
  • Though partly I'm just saying this because xlate makes me cry, but playerNameTranslationTable does too in all of its 26 character glory.
• Will you ever have any other name encodings? If so, it might be worth-while to consider making an interface and having these methods be non-static.
  • It would make your code a bit more verbose, but it would allow for a bit more decoupling
• Make 12 a constant
  • It would be nice to calculate it since maxLetters = floor(log(2^LONG_BITS) / log(37)), but there's no non-hideous way to calculate this
  • If it's a constant and you later expand the translation table, you'll only have to change 12 in one place instead of multiple. It's always good to reduce the chance of introducing future bugs.

---

Since the code is short, I gave it a go at revising it:

public class Review {

    private static char[] translationTable = buildTranslationTable();
    private static int maxLetters = 12;

    public static void main(String[] args)
    {
        System.out.println(playerNameToLong("Corbin92"));
        System.out.println(longToPlayerName(324533943486L));
    }

    public static String longToPlayerName(long name) {
        int i = 0;
        char[] nameCharacters = new char[maxLetters];
        final long base = translationTable.length;
        while (name != 0L) {
            final long ch = name % base;
            nameCharacters[maxLetters - ++i] = translationTable[(int) ch];
            name /= base;
        }
        return new String(nameCharacters, maxLetters - i, i);
    }

    public static long playerNameToLong(final String name) {
        final long base = translationTable.length;
        final int len = Math.min(name.length(), maxLetters);
        long l = 0L;
        for (int i = 0; i < len; ++i) {
            final char c = Character.toLowerCase(name.charAt(i));
            l *= base;
            if (c >= 'a' && c <= 'z') {
                l += 1 + (c - 'a');
            } else if (c >= '0' && c <= '9') {
                l += 27 + (c - '0');
            }
        }
        return l;
    }

    private static char[] buildTranslationTable()
    {
        char[] table = new char[37];
        for (char c = 'a'; c <= 'z'; ++c) {
            table[1 + c - 'a'] = c;
        }
        for (int i = 0; i < 10; ++i) {
            table[i + 27] = (char) (i + '0');
        }
        return table;
    }
}

Answer 2

You can directly convert an alphanumeric string to int64 and back in Java, using Long.parseLong and Long.toString using base 36. Like so:

System.out.println(Long.toString(994264677686L, 36));
System.out.println(Long.parseLong("Corbin92", 36));

What you then need to do is validation, for example check that the input string consists of alphanumeric characters; and handle edge cases, for example check that string is not too long or the number is not too small or too big etc.

Answer 3

Note: I realize both that this is old and that there is a more direct answer. But I think that knowing how and when to use a StringBuilder is more important in general than solving this problem.

public static String longToPlayerName(long name) {
    int i = 0;
    char[] nameCharacters = new char[12];
    while (name != 0L) {
        long ll = name;
        name /= 37L;
        nameCharacters[11 - i++] = playerNameXlateTable[(int) (ll - (name * 37L))];
    }
    return new String(nameCharacters, 12 - i, i);
}

This is what a StringBuilder is designed to do. Then you don't have to muck with internals like i. Consider

private static final char[] ALPHABET = " abcdefghijklmnopqrstuvwxyz0123456789".toCharArray();

public static String longToPlayerName(long packed) {
    StringBuilder reversed = new StringBuilder();
    for (; packed > 0L; packed /= ALPHABET.length) {
        reversed.append(ALPHABET[packed % ALPHABET.length]);
    }

    return reversed.reverse().toString();
}

No i to maintain. And it doesn't have to jump through hoops to get just the part of the array that represents the characters.

Doesn't have to hard code the length. Nor the size of the alphabet.

It's more obvious that the intermediate representation is reversed.

While I love math, each math operation is a potential error. This reduces the number of math operations to three (including the comparison). The original code had six (comparison, subtraction, increment, multiplication, subtraction, subtraction) and two hard coded constants.

And again, the base 36 solution is more appropriate to this particular problem. It's even a more compact encoding. But if you are writing a solution to a problem like this, a StringBuilder is a better tool than a character array.