-5

this is my issue. I am compiling with g++ (Ubuntu 7.3.0-27ubuntu1~18.04) 7.3.0

I would like to check every element of the string, and copy it only if it is a consonant.

This is how I try to do that: This is just a function to select vowels:

bool _is_vowel(char a){
    if (a=='a' || a=='A' || a=='e' || a=='E' ||a=='i' || a=='I' || a=='o' || a=='O' || a=='u' || a=='U' || a=='y' || a=='Y')
        return true;
    else return false;
}

This is the bit of code that does not work. Specifically, inside the ss.length() appears to be 0, and the string ss contains no character, but they are correctly printed inside the while loop.

 main(){
    int i=0, c=0; 
    string ss, name;
    cout << "insert name\n";
    getline(cin, name);
    while (i<int(name.length())){
       if (!_is_vowel(name[i])){
          cout << name[i]<< "\t";
          ss[c]=name[i];
          cout << ss[c]<< "\n";
          c++;
        }
        i++;
    }    

    cout << int(ss.length())<<endl;
    cout << ss;                
    }

Could anyone explain to me where I am wrong or give me some reference? Thank you in advance

Valentina Rolando
  • 9
  • 1
  • 5
    `ss[c]` accesses a character that does not exist. Use `ss += name[i]`. – Quentin Feb 20 '19 at 17:28
  • Enable compiler hints and warnings. Also, how are you accessing `s[c]` when you've given `s` no characters? What happens when you use the debugger to step through the code? – Ken White Feb 20 '19 at 17:28
  • Just `return a=='a' || a=='A' || a=='e' || a=='E' ||a=='i' || a=='I' || a=='o' || a=='O' || a=='u' || a=='U' || a=='y' || a=='Y';` and ditch the explicit `if` and `else`. – Jesper Juhl Feb 20 '19 at 17:28
  • Unrelated to your problem: You shouldn't use `_` as prefix for any symbols. That is preserved for c++ implementation. – πάντα ῥεῖ Feb 20 '19 at 17:29
  • @πάντα ῥεῖ I agree it's a bad idea. But, it's only reserved if followed by a second underscore or uppercase letter - not the case here. – Jesper Juhl Feb 20 '19 at 17:31
  • 5
    There is a much simplier way to handle this without any manual looping: `std::string ss = name; ss.erase(std::remove_if(ss.begin(), ss.end(), _is_vowel), ss.end());` – Remy Lebeau Feb 20 '19 at 17:32
  • 3
    @JesperJuhl Identifiers starting with a single leading `_` are also reserved, but at global scope only. – HolyBlackCat Feb 20 '19 at 17:34
  • @HolyBlackCat but, this is not at global scope (ok, hard to tell). So, I believe what I said is correct - in this context. – Jesper Juhl Feb 20 '19 at 17:36
  • 2
    @JesperJuhl `_is_vowel` seems to be declared at the global scope. – eerorika Feb 20 '19 at 17:42
  • 1
    @JesperJuhl • yes, I think you are correct, since `_is_vowel` is (as far as I can tell) at the global scope, so it is a reserved identifier. Technically, does that kind of violation qualify as _undefined behavior_ ? Or merely the potential for identifier collision by using a reserved identifier? – Eljay Feb 20 '19 at 17:42
  • 2
    @Eljay The standard specifies that defining a reserved identifier has undefined behaviour. The rule is there to avoid name collisions sure, but certain types of definitions, macros in particular, can have quite unpredictable outcome. – eerorika Feb 20 '19 at 17:50

2 Answers2

4
string ss;

Declares a string of length 0.

ss[c] = ...;

Then tries to access and set to an index that doesn't exist. This is Undefined Behavior. If you had used .at() which does bounds checking, you would've caught this immediately.

To fix this, you can either append to the string with the += operator:

ss += name[i];

Or you can use push_back():

ss.push_back(name[i]);

Furthermore, instead of checking every. single. character, I'd strongly suggest taking a look at this answer for a better way.

scohe001
  • 15,110
  • 2
  • 31
  • 51
0

The problem is in your while loop and it's easy to fix:

ss += name[i];

You simply need to concatenate to your string

Vic
  • 155
  • 1
  • 3
  • 12