0

I have this program 

int main()
{
    string valami = "-- .- .-. -.- ------ -- .- .-. -.-";
    from_morse_string(valami);
    return 0;
}

int from_morse_string(string input_morse_string)
{
    string morse_arr[1764];

    int j = 0;
    stringstream ssin(input_morse_string);
    while (ssin.good() && j < 1764)
    {
        ssin >> morse_arr[j];
        ++j;
    }

    for(int i = 0; i < j; i++)
    {
        switch(morse_arr[i])
        {
            case ".-" : cout << "a" << endl; break;
            case "-..." : cout << "b" << endl; break;
            case "-.-." : cout << "c" << endl; break;
            ...
            case "----." : cout << 9 << endl; break;
            case "-----" : cout << 0 << endl; break;
            default : cout << "it's contain invalid morse code";
        }
    }
    return 0;
}

It's a simple Morse code decoder, a very very simple program but when i want to run I get this error message : "switch quantity not an integer"

What's the problem? How can i solve it?

Improve this question
9
  • 3
    Aside: Never use .good() or .eof() as a loop condition. It almost always results in buggy code, as it does here. For a fixed length array, try while( j < 1764 && ssin >> morse_arr[j]) {j++;}. Commented Dec 6, 2013 at 18:25
  • @Robᵩ .good() is fine, as it is the same thing that is done when you do something like while (cin >> i) .... The bool operator for istream simply returns the good() check. Commented Dec 6, 2013 at 18:27
  • @ZacHowland - it's the same test, but it's done in a different place. while (std::cin >> i) will break out of the loop when the end of the input is reached. while (cin.good()) { cin >> i; ... } will execute the loop body once after the end of the input is reached. This typically shows up as a duplicated data item at the end of the input. Commented Dec 6, 2013 at 18:41
  • @ZacHowland: Looping on good() is a common but flawed practice. I see it so much that I keep the following question in my SO favorites: Testing stream.good() or !stream.eof() reads last line twice Commented Dec 6, 2013 at 18:46
  • @Blastfurnace My point was that good() is actually what is being called when you write while (cin >> i). It isn't so much that the function shouldn't be used to loop (as it should be), but how (that is, not directly). Commented Dec 6, 2013 at 18:51

6 Answers 6

Reset to default
11

What's the problem?

As the error says, you can only use switch with an integer, not a string.

How can i solve it?

With if and else:

if (morse_arr[i] == ".-") {
    cout << "a" << endl;
} else if (morse_arr[i] == "-...") {
    cout << "b" << endl;
} // and so on

or with a lookup table

std::map<std::string, char> morse_map = {
    {".-", 'a'},
    {"-...", 'b'},
    // and so on
};

auto found = morse_map.find(morse_arr[i]);
if (found == morse_map.end()) {
    cout << "invalid morse code\n";
} else {
    cout << found->second << endl;
}
Improve this answer
Sign up to request clarification or add additional context in comments.

Comments

3

The problem is that morse_arr is an array of strings and you can't use those as a condition for a switch statement.

Either use a series of if / else statements or compute a hash of the string and compare those (in a switch potentially) or some such.

Improve this answer

2 Comments

@Syngularity: What problem? switch statements work on integers, that is all. Just use a series of if/else if.
@Syngularity As I said, you could compute hash of the string and compare those, but using if / elses is a more straightforward approach.
1

I would like to point one flaw in your approach. You first compute all parts, and then map each part, whilst you could use an iterative approach to map each part as you discover it and thus need much less storage (and could loop indefinitely).

Because this seemed a fun problem, I decided to present here an alternative approach:

static std::map<std::string, char> const MorseMap = {
    {".-",    'a'},
    {"-...",  'b'},
    ...
};

void convert(std::istream& morse, std::ostream& regular) {
    std::string buffer;
    while (morse >> buffer) {
        auto const it = MorseMap.find(buffer);
        if (it == MorseMap.end()) { regular << '?'; continue; }

        regular << it->second;
    }
}

And yes, it works, and even remarks you have too many dashes as your fifth character.

In general, processing iteratively requires less memory, and yields more responsiveness, however it might complicate the design and slow down the overall computation; you have to judge whether it is worth it or not on a case by case basis.

Improve this answer

Comments

1

C++'s switch and case statements do not support comparing strings.

Comparing strings is not that efficient. When you have a fixed set of string values, I would normally choose to use integers or enums instead.

However, for your application, I would use a lookup table with the string and corresponding values. That's not more efficient, but it would be less code and easier to maintain.

Improve this answer

Comments

1

This is a simple problem to solve.

Translate dashes into the character '1' Translate dots into the character '2' Convert that value into an integer.

in your case statements replace the quoted string values by the actual integer number that it would translate to.

Sort your case statements so that the longer ones are found first.

Don't forget the colon : "---..."

--... ...-- de -.- .- ...-- -... --.. --.-

Improve this answer

2 Comments

if . is 0 and - is 1, then A .- would have the same value as U ..-
the case statements have to be sorted so as to encounter F ..-. before U ..- so that you match F instead of U. See Greedy Matching on wikipedia. You are thinking about the decoding from the input stream.
0

String data types cannot be used as a condition for switch because switch works correctly only on datatypes that can be converted to integer unambiguously. for example character data type can be converted to integer unambiguously and integer itself can be used as a condition for switch but string would be too ambiguous to be changed to integer that is why the error message pops up. You better try something else like 'if statements' to make your program work.

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.