searching through a char array

so far i have this, but the program just breaks at runtime. it only searches for A

#include <iostream>
#include <stdio.h>
#include <string.h>

using namespace std;

int main()
{
    char Input[13];
    char Output[13] = "1-800-";
    char Two[] = "AaBbCc";
    char Three[] = "DdCcEe";
    char Four[] = "GgHhIi";
    char Five[] = "JjKkLl";
    char Six[] = "MmNnOo";
    char Seven[] = "PpQqRrSs";
    char Eight[] = "TtUuVv";
    char Nine[] = "WwXxYyZz";
    int counter = 6;
    char *searching;
    char Two2[] = "2";

    cout << "Enter a phone number in the following format: 1-800-ABC-DEFG" << endl;
    cin >> Input;

    while(counter != 14)
    {
        searching = strchr(Input,'A');

        if(searching != NULL)
        {
            Output[counter] = 2;
        }

        counter++;
    }

    cout << Output << endl;

    return 0;
}

Jan 10, 2011 at 6:32pm

stereoMatching (308)

since you are using C++
you could consider std::string rather than char array before std::string could not satisfy your application

Last edited on Jan 10, 2011 at 6:33pm

Jan 10, 2011 at 8:44pm

yeah I probably should

Last edited on Jan 10, 2011 at 9:00pm

Jan 10, 2011 at 11:24pm

Duthomhas (13278)

You only need a lookup table.

1
2

                         /* ABCDEFGHIJKLMNOPQRSTUVWXYZ */
char ABC_to_123_lookup[] = "22233344455566677778889999";

If your input character is a letter, convert it to uppercase and subtract 'A' from it. That is the index of the correct digit in the lookup table.

I would also make a little function to do it for you, using the lookup table, and returning the translated character if it is alphabetic, or the same character if no lookup is needed (like for digits). I was going to just give it to you, but you can write it yourself. ;-)

Hope this helps.

Jan 10, 2011 at 11:28pm

I think Duoas has the best answer. Just do what he says and you're good to go.

Jan 11, 2011 at 2:37am

Yeah that sounds really good im gonna try that. well this is what i came up on my own though:

#include <iostream>
#include <string>

using namespace std;

int main()
{
    string Input;
    string Output = "1-800-";
    int counter = 6;
  
    cout << "Enter a phone number in the following format: 1-800-ABC-DEFG" << endl;
    cin >> Input;

    while(counter != 14)
        {
            if(Input[counter] == "A")//it wont let me compare this
            {
                Output = Output + "2";

                counter++;
            }

            if(counter == 9)
            {
                Output = Output + "-";

                counter++;
            }
        }

    cout << Output << endl;

    return 0;
}

Last edited on Jan 11, 2011 at 2:37am

Jan 11, 2011 at 2:40am

shadowvillian wrote:
`if(Input[counter] == "A")//it wont let me compare this`

Use single qoute.

Jan 11, 2011 at 2:42am

Use single quote.

correct you are.

Last edited on Jan 11, 2011 at 2:43am

Jan 11, 2011 at 2:57am

w00t I figured it out:

#include <iostream>
#include <string>

using namespace std;

int main()
{
    string Input;
    string Output = "1-800-";
    int counter = 6;

    cout << "Enter a phone number in the following format: 1-800-ABC-DEFG" << endl;
    cin >> Input;

    while(counter != 14)
        {
            if((Input[counter] == 'A')||(Input[counter] == 'B')||(Input[counter] == 'C'))
            {
                Output = Output + "2";

                counter++;
            }

            if((Input[counter] == 'D')||(Input[counter] == 'E')||(Input[counter] == 'F'))
            {
                Output = Output + "3";

                counter++;
            }

            if((Input[counter] == 'G')||(Input[counter] == 'H')||(Input[counter] == 'I'))
            {
                Output = Output + "4";

                counter++;
            }

            if((Input[counter] == 'J')||(Input[counter] == 'K')||(Input[counter] == 'L'))
            {
                Output = Output + "5";

                counter++;
            }

            if((Input[counter] == 'M')||(Input[counter] == 'N')||(Input[counter] == 'O'))
            {
                Output = Output + "6";

                counter++;
            }

            if((Input[counter] == 'P')||(Input[counter] == 'Q')||(Input[counter] == 'R')||(Input[counter] == 'S'))
            {
                Output = Output + "7";

                counter++;
            }

            if((Input[counter] == 'T')||(Input[counter] == 'U')||(Input[counter] == 'V'))
            {
                Output = Output + "8";

                counter++;
            }

            if((Input[counter] == 'W')||(Input[counter] == 'X')||(Input[counter] == 'Y')||(Input[counter] == 'Z'))
            {
                Output = Output + "9";

                counter++;
            }

            if(counter == 9)
            {
                Output = Output + "-";

                counter++;
            }
        }

    cout << Output << endl;

    return 0;
}

Jan 11, 2011 at 3:37am

Good for you. Also you might want to check out Duoas's idea.

#include <iostream>
#include <string>
#include <cctype>

using namespace std;

int main()
{
    string lookup = "22233344455566677778889999";
    string Input;
    string Output;

    cout << "Enter a phone number in the following format: 1-800-ABC-DEFG" << endl;
    cin >> Input;
    Output = Input;

    for (int i=0; i<Output.length(); i++)
    {
        if (isalpha(Output[i]))
        {
            char c = toupper(Output[i]);
            Output[i] = lookup[c-'A'];
        }
    }
    cout << Output << endl;
    return 0;
}

Jan 11, 2011 at 9:57am

rapidcoder (1010)

Indeed. :D

Funny how the original poster's code is better than the code posted by the experienced C++ forumers. Hint: there is a potential vulnerability in your code, blackcoder. Your code is safe only assuming standard ANSI C-locale. If the environment is configured with a different locale, the code can be easily forced to access the array out of its bounds. Also relying on specific ASCII coding of letters is not theoretically correct - because how can you be sure that the system is actually using ASCII? Maybe the code will be run on some exotic embedded system that has its "own encoding"?

Such things should be done with a hashmap. Lookup table is "clever" but this is the kind of cleverness that often makes software crash in the most unexpected moments.

Last edited on Jan 11, 2011 at 10:07am

Jan 11, 2011 at 11:22am

Duthomhas (13278)

The only fail going on here is nonsense like that.

The locale is not changed anywhere in the program, but if it is a concern, you can easily write your own safe version of isalpha. That, and C++ requires certain characteristics to hold true for the character set -- such as requiring that the set of English letters with the same letter case be sequentially arranged in the character table. Sorry EBCDIC.

But again, even if that is the case, a proper lookup is still correct, and, as is the case in the original code, does not assume ASCII:

char ABC_to_123( char c )
  {
  static const char lookupA[] = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
  static const char lookup0[] = "22233344455566677778889999";

  size_t index = std::find( lookupA, lookupA + 26, c ) - lookupA;

  return (index < 26) ? lookup0[ index ] : c;
  }

Hmm, what do you know? This is guaranteed to work on all random systems. Oh, notice how it is conservative with space -- a more important concern on embedded systems than your anti-cleverness nonsense.

"Such things should be done with a hashmap."
Are you kidding? Don't waste my time or memory. Wanna give me a hard time about excessive cleverness? You're a joke!

Oh, BTW, when have telephone keypads ever had letters other than "A-Z" on them? (Hint: never.)

Jan 11, 2011 at 12:12pm

[deleted]

Last edited on Jan 11, 2011 at 4:07pm

Jan 11, 2011 at 1:26pm

Well while my solution might be simplier, Duoas's is a much smaller amount of code. his might even be more simplier but thats what i was able to come up with being less experienced.

Jan 11, 2011 at 1:51pm

rapidcoder (1010)

@Duoas: your solution doesn't have the bug. I was not saying the lookup approach is generally bad, but by trying to make it short and clever (ok, the original code was long, I know), blackcoder introduced a very subtle bug into it. And finally the given code was worse than the original one. which was kinda funny, given the forum experience of blackcoder ;)

Many real vulnerabilities in programs are such small "almost-not-a-bug" things - they seem to be correct under some assumptions. But hackers like to attack in such a way to break these assumptions, thus breaking the code relying on them. Making the program correctness dependend on the codepage is one of such bugs waiting to happen. I've seen some Linux programs crashing when I changed the locale to UTF, so I've pointed this out. Be careful, and know what you are doing.

That, and C++ requires certain characteristics to hold true for the character set -- such as requiring that the set of English letters with the same letter case be sequentially arranged in the character table. Sorry EBCDIC.

Are you saying there are platforms where the C++ standard is not 100% implementable? Even worse: that there are platforms where C++ is not 100% implementable and Java is? (AS/400 for example that uses EBCDIC).

Are you kidding? Don't waste my time or memory

Sorry, but your corrected, flexible lookup is probably wasting much more cycles than the hashmap. :D And don't be kidding with the memory consumption. You saved like 50 bytes? Wow! You are an amazing optimizer! 50 bytes is nothing even on the most scarce in memory embedded systems.

BTW: Nice try with that lookup with STL, but it is still an ugly piece of code. That magic constants defending the end of the arrays are another possible problem waiting to happen (yes, someone removes a letter and voila, you have an off-by-one bug). I know I'm possibly niggling, but such small things too often show in Secunia reports. The original poster's code was still better and less ugly than that (ok, if was long, but anyway, what is the problem if something is long?)

Oh, BTW, when have telephone keypads ever had letters other than "A-Z" on them? (Hint: never.)

This is irrelevant to the problem I posted. This is like defending the old telnet buffer overflow vulnerability by saying "when does a remote terminal ever allow for more than 80 characters?"

Last edited on Jan 11, 2011 at 2:00pm

Jan 11, 2011 at 2:16pm

stereoMatching (308)

maybe multimap of the stl could make the problem easier and safer?
it is a sorry that C++ don't have any built in hash table yet

Jan 11, 2011 at 2:50pm