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

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

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

Problem

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 = 'A') && (c = 'a') && (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!

Solution

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 = 'a' && 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;
    }
}

Code Snippets

final int base = playerNameXlateTable.length;
while (name != 0L) {
    final long ch = name % base;
    nameCharacters[11 - i++] = playerNameXlateTable[ch];
    name /= base;
}
if (c >= 'A' && c <= 'Z') {
1 + (c - 'a')
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;
    }
}

Context

StackExchange Code Review Q#28264, answer score: 6

Revisions (0)

No revisions yet.