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

Creating a numeric phone keypad

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

Problem

I'm writing a Telephone class and it has a method called getDigits which takes in a String parameter and returns the number that would appear on your phone if you typed in those letters the old fashioned way.

Example: Typing in "CAT" would return 228.

I wrote the following code and it works but I wanted to know if anyone else out there had better ideas to do it/ more sophisticated ones. I'm learning about data abstraction/enums and maybe that's a clue to how my professor wanted me to approach this but I'm stumped.

public int getDigits(String test){
        String result = "";
        String param = test.toUpperCase();
        for(int i = 0; i<param.length(); i++){
            String s = Character.toString(param.charAt(i));
            if(s.equals("A") || s.equals("B") || s.equals("C")){
                result += 2;
            }

            else if(s.equals("D") || s.equals("E") || s.equals("F")){
                result += 3;
            }

            else if(s.equals("G") || s.equals("H") || s.equals("I")){
                result += 4;
            }

            else if(s.equals("J") || s.equals("K") || s.equals("L")){
                result += 5;
            }

            else if(s.equals("M") || s.equals("N") || s.equals("O")){
                result += 6;
            }

            else if(s.equals("P") || s.equals("Q") || s.equals("R") || s.equals("S")){
                result += 7;
            }
            else if(s.equals("T") || s.equals("U") || s.equals("V")){
                result += 8;
            }

            else if(s.equals("W") || s.equals("X") || s.equals("Y") || s.equals("Z")){
                result += 9;
            }
        }
        return Integer.parseInt(result);
    }

Solution

The text-to-key problem is one that has multiple solutions. The Map solution is a good one, there's nothing wrong with it. There are two other algorithms that are good too. First, though, please convert the char-to-key to a function. Having such a big loop is a problem.

public static int charToKey(char c) {
    ... do something to return key ....
}


A switch statement is similar to your cascading if's, except it is neater, and faster:

switch (c) {
    case 'A': case 'B': case 'C':
        return 2;
    case 'D': case 'E': case 'F':
        return 3;
    ....
}


Note that I abuse the layout of the switch to have multiple chars on a line.... I sometimes do that for structured inputs like this. Also, note that a case block with a return statement does not need a break - that's really nice when putting switches in to functions - just return, don't break.

A second alternative to a Map, is a mathematical function. The math one is interesting.... because it is fast. It's not that readable, though.....

Consider your keys, most keys have 3 letters, but 7 and 9 have 4 letters. If we treat Z as a special case, we can think of 9 as having just 3, which leaves only key 7 with 4 letters. Now, if we lay those letters on to a number line, where each letter occupies just... 0.32 of a span (we choose 0.32 because 4 of them is just slightly less than 1), and we organize that P is aligned with 7.0 ..... and we also take the integer value of the result... we have:

double span = 0.32;
    double offset = 7.0 - span * ('P' - 'A');
    for (char c = 'A'; c  %.3f ==> %d\n", c, val, key);
    }


and that produces:

A -> 2.200 ==> 2
B -> 2.520 ==> 2
C -> 2.840 ==> 2
D -> 3.160 ==> 3
E -> 3.480 ==> 3
F -> 3.800 ==> 3
G -> 4.120 ==> 4
H -> 4.440 ==> 4
I -> 4.760 ==> 4
J -> 5.080 ==> 5
K -> 5.400 ==> 5
L -> 5.720 ==> 5
M -> 6.040 ==> 6
N -> 6.360 ==> 6
O -> 6.680 ==> 6
P -> 7.000 ==> 7
Q -> 7.320 ==> 7
R -> 7.640 ==> 7
S -> 7.960 ==> 7
T -> 8.280 ==> 8
U -> 8.600 ==> 8
V -> 8.920 ==> 8
W -> 9.240 ==> 9
X -> 9.560 ==> 9
Y -> 9.880 ==> 9
Z -> 10.200 ==> 10


Note how all the characters have valid values, except Z. Now, there's a trick with that... which is as ugly as anything, but it works really well.... key -= key/10 - subtract an integral 10'th of the value from itself.

What does this mean? Well, it means that your function can be (for valid input):

public static int charToKey(char c) {
    int key = (int)(2.2 + (c - 'A') * 0.32);
    return key - key / 10;
}


If you want to validate the input, something like:

char uc = Character.toUpperCase(c) - 'A';
if (uc = 26) {
    return 0;
}
int key = (int)(2.20 + uc * 0.32);
return key - key / 10;


Update

Since this became a benchmarking session comparing various options, and since a new option came up in discussions with @chillworld, it seems an update is in order.

First up, I was reminded that this:

int length = text.length();
for (int i = 0; i < length; i++) {....


is faster than:

for (int i = 0; i < text.length(); i++) {....


I checked that, and, in fact, it is true. I consistently get the following benchmark times:

Task Keys -> Calc: (Unit: MILLISECONDS)
  Count    :     1000      Average  :   0.2914
  Fastest  :   0.2749      Slowest  :   2.3255
  95Pctile :   0.3327      99Pctile :   0.4448
  TimeBlock : 0.315 0.285 0.292 0.285 0.284 0.280 0.293 0.283 0.299 0.297
  Histogram :   999     0     0     1

Task Keys -> Cached: (Unit: MILLISECONDS)
  Count    :     1000      Average  :   0.2802
  Fastest  :   0.2665      Slowest  :   1.9090
  95Pctile :   0.3212      99Pctile :   0.4237
  TimeBlock : 0.298 0.275 0.284 0.276 0.276 0.269 0.284 0.274 0.283 0.284
  Histogram :   999     0     1


Where the different code paths are just:

public static int calced(String text) {
    int result = 0;
    for (int i = 0; i < text.length(); i++) {
        result *= 10;
        result += charToKey(text.charAt(i));
    }
    return result;
}

public static int cached(String text) {
    int result = 0;
    final int len = text.length();
    for (int i = 0; i < len; i++) {
        result *= 10;
        result += charToKey(text.charAt(i));
    }
    return result;
}


"Caching" the String length has a small (1% in this case) improvement in performance.

More interestingly, the idea @Chillworld had of creating a lookup array for each relevant character is much, much faster. His idea started off as:

private static final int[] cache = new int[91]; 
static { 
    cache[65] = 2; 
    cache[66] = 2; 
    cache[67] = 2; 
    cache[68] = 3; 
    cache[69] = 3; 
    cache[70] = 3; 
    ....


Essentially, build an array with a direct 1-to-1 mapping between the char value, and the phone key number.

Until this point, the fastest transform was by doing a calculation, but comparing that calculation with:

```
private static int cached(String text) {
int result = 0;
for (int i = 0; i < text.length(); i++) {

Code Snippets

public static int charToKey(char c) {
    ... do something to return key ....
}
switch (c) {
    case 'A': case 'B': case 'C':
        return 2;
    case 'D': case 'E': case 'F':
        return 3;
    ....
}
double span = 0.32;
    double offset = 7.0 - span * ('P' - 'A');
    for (char c = 'A'; c <= 'Z'; c++) {
        double val = offset + span * (c - 'A');
        int key = (int)val;
        System.out.printf("%s -> %.3f ==> %d\n", c, val, key);
    }
A -> 2.200 ==> 2
B -> 2.520 ==> 2
C -> 2.840 ==> 2
D -> 3.160 ==> 3
E -> 3.480 ==> 3
F -> 3.800 ==> 3
G -> 4.120 ==> 4
H -> 4.440 ==> 4
I -> 4.760 ==> 4
J -> 5.080 ==> 5
K -> 5.400 ==> 5
L -> 5.720 ==> 5
M -> 6.040 ==> 6
N -> 6.360 ==> 6
O -> 6.680 ==> 6
P -> 7.000 ==> 7
Q -> 7.320 ==> 7
R -> 7.640 ==> 7
S -> 7.960 ==> 7
T -> 8.280 ==> 8
U -> 8.600 ==> 8
V -> 8.920 ==> 8
W -> 9.240 ==> 9
X -> 9.560 ==> 9
Y -> 9.880 ==> 9
Z -> 10.200 ==> 10
public static int charToKey(char c) {
    int key = (int)(2.2 + (c - 'A') * 0.32);
    return key - key / 10;
}

Context

StackExchange Code Review Q#104879, answer score: 10

Revisions (0)

No revisions yet.