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

Optimize text to ascii lookup map and scalability

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

Problem

Is there a more optimal way of retrieving the character for the letter? I would assume that I would just replace my Entry set loop with another map, but I do not want to bloat the code, because it will reduce scalability.

```
import java.util.HashMap;
import java.util.Map;
import java.util.Map.Entry;

public class AciiLetters {
private enum Letter {
_A("010101111101101"), _B("110101110101110"), _C("011100100100011"),
_D("110101101101110"), _E("111100110100111"), _F("111100110100100"),
_G("011100101101011"), _H("101101111101101"), _I("111010010010111"),
_J("011001001101010"), _K("101101110101101"), _L("100100100100111"),
_M("101111111101101"), _N("111101101101101"), _O("010101101101010"),
_P("110101110100100"), _Q("010101101011001"), _R("110101110101101"),
_S("011100010001110"), _T("111010010010010"), _U("101101101101111"),
_V("101101101101010"), _W("101101111111101"), _X("101101010101101"),
_Y("101101010010010"), _Z("111001010100111"), _0("111101101101111"),
_1("010110010010111"), _2("111001111100111"), _3("111001011001111"),
_4("101101111001001"), _5("111100111001111"), _6("111100111101111"),
_7("111001001001001"), _8("111101111101111"), _9("111101111001111"),
SPACE("000000000000000"), UNKNOWN("010101001010010");

private static final Map cache;
static {
cache = new HashMap();

cache.put('A', _A); cache.put('B', _B); cache.put('C', _C);
cache.put('D', _D); cache.put('E', _E); cache.put('F', _F);
cache.put('G', _G); cache.put('H', _H); cache.put('I', _I);
cache.put('J', _J); cache.put('K', _K); cache.put('L', _L);
cache.put('M', _M); cache.put('N', _N); cache.put('O', _O);
cache.put('P', _P); cache.put('Q', _Q); cache.put('R', _R);
cache.put('S', _S); cache.put('T', _T); cache.put('U', _U);
cache.put('V', _V); cache.put('W', _W); cache.p

Solution

This is one of those occasions where a 'sparse' dataset may be very helpful.

But, first some nit-picks:

  • You should probably call the class AsciiLetters instead of AciiLetters



-
Declaring multiple constants on one line is unconventinal, and makes things hard to read... fix:

public static final int WIDTH = 3, HEIGHT = 5;


-
In the printText you over-cook the lookups to the Enum:

for (int row = 0; row < Letter.HEIGHT; row++) {
    for (Character tCh : text.toCharArray()) {
        letter = Letter.getEnum(tCh);


this can become:

char[] textchars = text.toCharArray();
Letter[] letters = new Letter[textchars.length];
for (int i = 0; i < textchars.length; i++) {
    letters[i] = Letter.getEnum(textchars[i]);
}
for (int row = 0; row < Letter.HEIGHT; row++) {
    for (letter letter : letters) {


-
also in the printText(), there is no need to do all the work with Character. Using char is better.

  • also, declaring all your variables outside the loop is not useful for anything, and can even make performance worse. Languages like C need that, but Java is better if you declare your variables when you need them, not before.



  • If you wanted to, you could store the 0 and 1 values as actual bits. This would actually be faster (slightly) but would also be more complicated. In this case, I don't think it is significantly different.



OK, now for the real issue, the storage of the Enums in a convenient-to-access system...

First, I recommend that you change your Enum to have two parameters in the constructor:

private String bitSequence;
    Letter(String bitSequence) {
        this.bitSequence = bitSequence;
    }


This should become (note, I have also made them final!):

private final String bitSequence;
    private final char mychar;
    Letter(char c, String bitSequence) {
        this.mychar = c;
        this.bitSequence = bitSequence;
    }


Then change each of your Enum values to also send the char it represents.

If you do that, then your getChar() method becomes simply:

public char getChar() {
        return mychar;
    }


BUT, I was lazy, and I pulled your code in to my eclipse environment, and I could not be bothered to change all the Enum values... so I cheated... and used:

public char getChar() {
        return name().charAt(1);
    }


That takes the second letter from the Enum name (e.g. it will pull L from _L). Since all your Enum names have a systematic name scheme, this will work, but, it's not the best system...

But still, once I have the getChar() working off the internal values of the enum (instead of the Map), you can do the following:

replace the line:

private static final Map cache;


and instead have:

// store enough values for ASCII characters. If we wanted to, even 32768 is not large
// and with that we could store **all** Letters,
private static final Letter[] cache = new Letter[128];


Then, in your static block, fill the cache with 'Unknown', and then 'fix' the ones you know:

static {
        // assume things are UNKNOWN.
        Arrays.fill(cache, UNKNOWN);
        // 'fix' the things we actually know...
        for (Letter letter : values()) {
            // use the actual char (cast implicitly to an int...)
            // as the index to the array.
            // if you have chars >= 128 you will need to make the array bigger.
            cache[letter.getChar()] = letter;
        }
    }


Finally, using this array is really simple:

public static Letter getEnum(char value) {
        return value < cache.length ? cache[value] : UNKNOWN;
    }


Note, I have changed the parameter to simple char instead of Character.

Putting it all together, I have your code running as (without the additional char as a constructor to the Enum...):

```
import java.util.Arrays;

public class AsciiLetters {
private enum Letter {
_A("010101111101101"), _B("110101110101110"), _C("011100100100011"),
_D("110101101101110"), _E("111100110100111"), _F("111100110100100"),
_G("011100101101011"), _H("101101111101101"), _I("111010010010111"),
_J("011001001101010"), _K("101101110101101"), _L("100100100100111"),
_M("101111111101101"), _N("111101101101101"), _O("010101101101010"),
_P("110101110100100"), _Q("010101101011001"), _R("110101110101101"),
_S("011100010001110"), _T("111010010010010"), _U("101101101101111"),
_V("101101101101010"), _W("101101111111101"), _X("101101010101101"),
_Y("101101010010010"), _Z("111001010100111"), _0("111101101101111"),
_1("010110010010111"), _2("111001111100111"), _3("111001011001111"),
_4("101101111001001"), _5("111100111001111"), _6("111100111101111"),
_7("111001001001001"), _8("111101111101111"), _9("111101111001111"),
SPACE("000000000000000"), UNKNOWN("010101001010010");

public static final int WIDTH = 3;
public static final int HEIGHT = 5;

Code Snippets

public static final int WIDTH = 3, HEIGHT = 5;
for (int row = 0; row < Letter.HEIGHT; row++) {
    for (Character tCh : text.toCharArray()) {
        letter = Letter.getEnum(tCh);
char[] textchars = text.toCharArray();
Letter[] letters = new Letter[textchars.length];
for (int i = 0; i < textchars.length; i++) {
    letters[i] = Letter.getEnum(textchars[i]);
}
for (int row = 0; row < Letter.HEIGHT; row++) {
    for (letter letter : letters) {
private String bitSequence;
    Letter(String bitSequence) {
        this.bitSequence = bitSequence;
    }
private final String bitSequence;
    private final char mychar;
    Letter(char c, String bitSequence) {
        this.mychar = c;
        this.bitSequence = bitSequence;
    }

Context

StackExchange Code Review Q#37392, answer score: 3

Revisions (0)

No revisions yet.