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

Compare the index of each char in a string in an alphabet array to a range of numbers

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

Problem

I saw this codegolf challenge and I set out to try and write a solution for it. I'm nowhere near an expert codegolfer (or programmer), but it was an interesting exercise.

Now I'm wondering how to improve my code, as it feels really bulky (especially compared to some of the answers to the challenge). Note that I'm not looking for ways to golf this code, I'm merely looking for general improvements and optimizations, hints and tips.

I hope the title is clear as to what the program does, it was pretty hard to describe!

Anyway, what my program does is pretty simple:

  • It accepts a string as input



  • It then fetches the index of each char in the string from the array



  • It checks if that char is within a certain range



  • Each range corresponds with a classic mobile phone button (0,1,2 = A,B,C)



  • The current button is compared to the previously used button



  • If the buttons match, the string does not pass and it returns false



My code:

int previousButton = -1;

char[] letters =
{
    'a', 'b', 'c',       // 2
    'd', 'e', 'f',       // 3
    'g', 'h', 'i',       // 4
    'j', 'k', 'l',       // 5
    'm', 'n', 'o',       // 6
    'p', 'q', 'r', 's',  // 7
    't', 'u', 'v',       // 8
    'w', 'x', 'y', 'z',  // 9
    ' '                  // 0
};

for (int i = 0; i = 0 && index = 3 && index = 6 && index = 9 && index = 12 && index = 15 && index = 19 && index = 22 && index <= 25) {
        currentButton = 9;
    } else if (index == 26) {
        currentButton = 0;
    }

    if (previousButton == currentButton) {
        return false;
    }

    previousButton = currentButton;
}

return true;

Solution

There are two alternatives I can recommend, one alternative avoids all the if/else/if cascading, and replaces it with a single 'switch' statement. Switch statements are optimized at compile time so that, effectively, each char/operation takes as long as any other. it makes sense to extract that to a function too.

The second alternative is to trade code space, for memory space.

First though, improving your current solution...
Current version

Your current version has a lot of unnecessary conditions in the cascading if/else system.

If you check for 'low' values first, then you can assume the next value's lower range is already handled. It is easier to explain this by example.... you have:

if (index >= 0 && index = 3 && index = 6 && index <= 8) {


But this will produce the same results, with half the comparisons:

if (index < 0)
{
    currentButton = 1; //invalid index, char not found?
}
else if (index <= 2)
{
    currentButton = 2;
}
else if (index <= 5)
{
    currentButton = 3;
}
else if (index <= 8)
{


Note how I have also used the conventional C# style there for the braces....
  1. Switch:



A switch statement can work on chars:

switch (currentLetter)
{
    case 'a':
    case 'b':
    case 'c':
        return 2;
    case 'd':
    case 'e':
    case 'f':
        return 3;
    .....
    default:
        return 1; // whatever you need for an invalid char
}


The above code (included in a function) will encode each char to a key, and unmapped chars will return 1.
  1. In memory lookup



A second common way to do this is to prepopulate an array with the indexes for each char:

int[] keys = new int[]{2,2,2,3,3,3,4,4,4,5,5,5,6,6,6,7,7,7,7,8,8,8,9,9,9,9};


Then use that array as a simple lookup:

if (currentChar  'z')
{
    return 1;
}
return keys[currentChar - 'a'];


That converts the char to an offset in the keys array. I have taken the liberty of coding this up in Ideone to ensure it works (which it does, except for the handling of space characters)

Code Snippets

if (index >= 0 && index <= 2) {
    currentButton = 2;
} else if (index >= 3 && index <= 5) {
    currentButton = 3;
} else if (index >= 6 && index <= 8) {
if (index < 0)
{
    currentButton = 1; //invalid index, char not found?
}
else if (index <= 2)
{
    currentButton = 2;
}
else if (index <= 5)
{
    currentButton = 3;
}
else if (index <= 8)
{
switch (currentLetter)
{
    case 'a':
    case 'b':
    case 'c':
        return 2;
    case 'd':
    case 'e':
    case 'f':
        return 3;
    .....
    default:
        return 1; // whatever you need for an invalid char
}
int[] keys = new int[]{2,2,2,3,3,3,4,4,4,5,5,5,6,6,6,7,7,7,7,8,8,8,9,9,9,9};
if (currentChar < 'a' || currentChar > 'z')
{
    return 1;
}
return keys[currentChar - 'a'];

Context

StackExchange Code Review Q#62637, answer score: 8

Revisions (0)

No revisions yet.