4
\$\begingroup\$

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

Yes. this is client code (from the client). I can't seem to come up with a better way. Any ideas would be excellent!

\$\endgroup\$
6
\$\begingroup\$

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;
    }
}
\$\endgroup\$
  • \$\begingroup\$ Wow! Thanks for the excellent tips! :- ) \$\endgroup\$ – rmcmk Jul 10 '13 at 2:53
  • \$\begingroup\$ @AtomicInt_ Glad I could help! If possible at all, by the way, you should try to use abuzittin gillifirca's approach. It uses a different encoding with different properties, so it might not be possible, but what your code is essentially changing bases, which his done in 2 lines :). \$\endgroup\$ – Corbin Jul 11 '13 at 18:44
5
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Hmmm, well that's awkward.... Seems my Java rustiness is showing :). \$\endgroup\$ – Corbin Jul 9 '13 at 8:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.