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?

Zong
  • 6,160
  • 5
  • 32
  • 46
Syngularity
  • 795
  • 3
  • 17
  • 42
  • 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++;}`. – Robᵩ Dec 06 '13 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. – Zac Howland Dec 06 '13 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. – Pete Becker Dec 06 '13 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](http://stackoverflow.com/questions/4324441/testing-stream-good-or-stream-eof-reads-last-line-twice) – Blastfurnace Dec 06 '13 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). – Zac Howland Dec 06 '13 at 18:51
  • @ZacHowland: I thought it's actually using `fail()` in that situation. See: [std::basic_ios::operator bool](http://en.cppreference.com/w/cpp/io/basic_ios/operator_bool) – Blastfurnace Dec 06 '13 at 19:06
  • @Blastfurnace There is a table at the bottom of the [`fail()`](http://en.cppreference.com/w/cpp/io/basic_ios/fail) documentation, but I'm not sure how accurate it is. It has `good` returning `true` when the eofbit is set. I'd have to double check the standard, but I'm pretty sure it is suppose to return `false` for that. Other than that, it has the identical return values as `operator bool()`. – Zac Howland Dec 06 '13 at 19:11
  • @ZacHowland: In my copy of the standard (N3690, 27.5.5.4) `std::ios_base::operator bool()` returns `!fail()`. I think the loop condition stops due to failed stream extraction, not eof. Either way, using `good()` as a loop condition is bad() – Blastfurnace Dec 06 '13 at 19:20
  • @Blastfurnace In that case, I stand corrected. – Zac Howland Dec 06 '13 at 19:26

6 Answers6

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;
}
Mike Seymour
  • 249,747
  • 28
  • 448
  • 644
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.

ScarletAmaranth
  • 5,065
  • 2
  • 23
  • 34
  • 1
    @Syngularity: What problem? switch statements work on integers, that is all. Just use a series of if/else if. – Ed S. Dec 06 '13 at 18:22
  • 1
    @Syngularity As I said, you could compute hash of the string and compare those, but using if / elses is a more straightforward approach. – ScarletAmaranth Dec 06 '13 at 18:22
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.

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
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 -.- .- ...-- -... --.. --.-

EvilTeach
  • 28,120
  • 21
  • 85
  • 141
  • if . is 0 and - is 1, then A .- would have the same value as U ..- – EvilTeach Dec 06 '13 at 20:51
  • 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. – EvilTeach Dec 06 '13 at 20:52
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.

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
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.

Ephrem F.
  • 1