3

Hello i don't know how to fix my program to find how many evens, odds, vowels and consonants are in my string.

The program compiles and runs but i never get any vowels or consonants and everything else is added in either evens or odds (even characters).

Edit 1: By evens and odds i mean like if the user types in the string John123 i want to find how many characters are vowels in this case 1 the 'o' how many are consonants in this case 3 the 'J', 'h', 'n' how many are evens in this case 1 the '2' and how many are odds in this case 2 the '1' and the '3'.

#include <iostream>
#include <string>

using namespace std;

int main ()
{
    string s1;
    int evens = 0; //
    int odds = 0; // 
    int vowels = 0; //
    int consonants  = 0; // 
    cout << "Please type in Something: ";
    getline(cin, s1);
    cout << "You typed: " << s1 << endl;

    for(int i = 0; i < s1.size(); ++i)
    {
        if((s1[i] % 2) == 0 ) 
        {
            ++evens;
        }
        else if((s1[i] % 2 ) != 0) // What would an algorithm (formula) be to find when a number is odd without the not-equals != ?
        {
            ++odds;
        }

        else if(s1[i] == 'A' || 'a' || 'E' || 'e' || 'O' || 'o' || 'I' || 'i' || 'U' || 'u')
        {
            ++vowels;
        }
        else if(s1[i] == 'Q' || 'W' || 'R' || 'T' || 'Y' || 'P' || 'S' || 'D' || 'F' || 'G' || 'H' || 'J' || 'K' || 'L' || 'Z' || 'X' || 'C' || 'V' || 'B' || 'N' || 'M'||
                         'q' || 'w' || 'r' || 't' || 'y' || 'p' || 's' || 'd' || 'f' || 'g' || 'h' || 'j' || 'k' || 'l' || 'z' || 'x' || 'c' || 'v' || 'b' || 'n' || 'm')
 //   I specify every letter so that it dosn't pick whitespaces, symbols etc.
        {
            ++consonants;
        }
    }
    cout << "Evens in string are: " << evens << "\nOdds in string are: " << odds << "\nVowels in string are: " << vowels << "\nConsonants in string are: " << consonants;

    return 0;
}
Johnson
  • 145
  • 4
  • 10
  • 2
    Your `OR` syntax is way, way off. Do you have any notes or a text book? Check them again. – Jongware Dec 11 '14 at 16:24
  • Unfortunately no my friend as i am self learning through youtube lectures tutorials etc. Why is my OR syntax wrong? Do you mean that there is a faster more readable way to do it? if yes thats what i want to know :P. – Johnson Dec 11 '14 at 16:26
  • Short answer: `s1[i] == 'A' || s1[i] == 'a' || s1[i] == ..`. Long answer: look up "Binary operators in C++". – Jongware Dec 11 '14 at 16:28
  • What do you mean with "even" and "odd"? Are you referring to the numerical position in the string or in the alphabet or maybe the even/oddness of the numerical representation in the ASCII character set table? – Felix Glas Dec 11 '14 at 16:55
  • By evens and odds i mean like if the user types in the string John123 i want to find how many characters are vowels in this case 1 the 'o' how many are consonants in this case 3 'J', 'h', 'n' how many are evens in this case 1 the '2' and how many are odds in this case 2 the '1' and the '3'. – Johnson Dec 11 '14 at 17:03
  • @Johnsonvdg You should edit this into your question for clearification. – Felix Glas Dec 11 '14 at 17:06
  • Ok will do so right now thanks. – Johnson Dec 11 '14 at 17:12

6 Answers6

3
if((s1[i] % 2) == 0 )

This code is not correct. You are modulating the character, which isn't what you want. You want to first convert the character to an integer using atoi or something like that, I can't remember the function. (Just look up string to int c++). Then, once you've converted the character to an integer, then you can modulate it correctly.

Also, your || syntax is totally wrong, as Jongware mentioned. You need to have an == comparison for each character.

Should look like this (very long):

(s1[i] == 'A' || s1[i] == 'a' || s1[i] == 'E' || s1[i] == 'e' || s1[i] == 'O' || s1[i] == 'o' || s1[i] == 'I' || s1[i] == 'i' || s1[i] == 'U' || s1[i] == 'u')

You might want to store s1[i] in a char beforehand so it's more concise.

And you should place your character testing BEFORE the odd/even testing, so that you don't accidentally try and convert h or another non-numeric character to an integer.

BWG
  • 2,238
  • 1
  • 20
  • 32
3

A char is a number. For example, a is (on most computers) the ASCII code 97. So either your even test or your odd test will always match. You need to rearrange your if blocks.

Also, your character tests don't do what you intended. s1[i] == 'A' || 'a' tests as true whenever s1[i] is equal to 'A' or whenever 'a' as a number is not zero. But 97 is never zero, so that test will always result in true. You need code more like s1[i] == 'A' || s1[i] == 'a', testing the s1[i] value each time.

(One way to make things simpler: Once you know the character is not a vowel, you can test for an ASCII consonant with ((s1[i] >= 'a' && s1[i] <= 'z') || (s1[i] >= 'A' && s1[i] <= 'Z')).)

aschepler
  • 70,891
  • 9
  • 107
  • 161
  • Thanks my friend. One question tho. Wouldnt this: ((s1[i] >= 'a' && s1[i] <= 'z') || (s1[i] >= 'A' && s1[i] <= 'Z')) ALso check for vowels for example? because checking from a to z as i am understanding it also checks vowels too and vice versa – Johnson Dec 11 '14 at 16:40
  • Yes, it would check true for all ASCII letters - so you would use it in combination with a vowel check. – aschepler Dec 11 '14 at 16:43
2

Your || syntax is making no sence, what you currently have is something like this

else if(s1[i] == 'A' || 'a' || 'E' || 'e' || 'O' || 'o' || 'I' || 'i' || 'U' || 'u')

while it should be like this:

else if(s1[i]=='A' || s1[i]=='a' || s1[i]=='E' || s1[i]=='e' || s1[i]=='O' || s1[i]=='o' || s1[i]=='I' || s1[i]=='i' || s1[i]=='U' || s1[i]=='u')
2

You have basically two problems:

  1. Characters are interpreted as numbers, taking into account the character code. When you are checking whether the character is even or odd, you are checking its character code which will always be either event of odd, this is why your code will always enter into the first or second case. You must check whether your character is digit and if so, you must check whether it is even

  2. Your OR is incorrect, see Ruby van Soelen's answer

Suggested, untested code:

for(int i = 0; i < s1.size(); ++i)
{
    if (isdigit(s1[i])) {
        if ((s1[i] == '0') || (s1[i] == '2') || (s1[i] == '4') || (s1[i] == '6') || (s1[i] == '8')) 
        {
            ++evens;
        }
        else // What would an algorithm (formula) be to find when a number is odd without the not-equals != ?
        {
            ++odds;
        }
    }
    else 
    {

        if(s1[i]=='A' || s1[i]=='a' || s1[i]=='E' || s1[i]=='e' || s1[i]=='O' || s1[i]=='o' || s1[i]=='I' || s1[i]=='i' || s1[i]=='U' || s1[i]=='u')
        {
            ++vowels;
        }
        else if(s1[i]=='B' || s1[i]=='b' || s1[i]=='C' || s1[i]=='c' || s1[i]=='D' || s1[i]=='d' || s1[i]=='F' || s1[i]=='f' || s1[i]=='G' || s1[i]=='g' || s1[i]=='H' || s1[i]=='h' || s1[i]=='J' || s1[i]=='j' || s1[i]=='K' || s1[i]=='k' || s1[i]=='L' || s1[i]=='l' || s1[i]=='M' || s1[i]=='m' || s1[i]=='N' || s1[i]=='n' || s1[i]=='P' || s1[i]=='p' || s1[i]=='Q' || s1[i]=='q' || s1[i]=='R' || s1[i]=='r' || s1[i]=='S' || s1[i]=='s' || s1[i]=='T' || s1[i]=='t' || s1[i]=='V' || s1[i]=='v' || s1[i]=='X' || s1[i]=='x' || s1[i]=='Y' || s1[i]=='y' || s1[i]=='Z' || s1[i]=='z')
 //   I specify every letter so that it dosn't pick whitespaces, symbols etc.
        {
            ++consonants;
        }
    }
}
Lajos Arpad
  • 64,414
  • 37
  • 100
  • 175
  • Thanks everyone for helping me but this method seemed the easiest hence best answer. But my friend you should remove the vowels and consonants part from the code cause actually both what you typed do the same checks with vowels ( the last of course is not valid but still :)) – Johnson Dec 11 '14 at 17:19
  • You are right, I have made a few fixes. We need a white list for consonants, so characters, like ./'" and so on will not be counted as consonants. – Lajos Arpad Dec 11 '14 at 17:47
  • Yea that's why i included and OR-ed with every letter and not used a simple else. Because it would include white-spaces, periods, commas etc.... also you could change the owels check because it was the wrong way i used :P – Johnson Dec 11 '14 at 18:40
  • 1
    Yes, you are right. I am happy thought that these inaccuracies did not prevent you from understanding the point of my answer. Cheers! – Lajos Arpad Dec 11 '14 at 19:00
1

Expression in the else-if statement will be always equal to true

else if(s1[i] == 'A' || 'a' || 'E' || 'e' || 'O' || 'o' || 'I' || 'i' || 'U' || 'u')

becuase it is equivalent to

( s1[i] == 'A'  ) || 'a' || 'B' /*...*/

where 'a' as it is not equal to zero evaluates to true.

So you have to write at least like

s1[i] == 'A' || s1[i] == 'a' || s1[i] == 'B' /*...*/

Also take into account that for example character 'B' though is not a digit but its code is even. So you will get a wrong result if will start checks from statement

if((s1[i] % 2) == 0 ) 

You should check at first whether the character is a digit. Also you could define an array of vowels and consonants and use standard function strchr that to check whether a character is a vowel or consonant.

For example

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

//...

const char *alpha_vowels = "AEIOU";
const char &alpha_consonants = "BCD...";

//...

if ( std::isdigit( s[i] ) )
{
    s[i] % 2 == 0 ? ++evens : ++odds;
}
else if ( std::strchr( alpha_vowels, std::toupper( ( unsigned char )s[i] ) ) != nullptr )
{
    ++vowels;
}
else if ( std::strchr( alpha_consonants, std::toupper( ( unsigned char )s[i] ) ) != nullptr )
{
    ++consonants;
}
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
  • Thanks my friend for your answer. But it seems you are using pointer which i am not aware how to use at this stage :P and i dont want to copy paste your code because i want to learn .:) so i will try the others ways but still thanks for your help. – Johnson Dec 11 '14 at 17:08
  • @Johnsonvdg I removed the pointer declarations. See my updated post. – Vlad from Moscow Dec 11 '14 at 17:20
1

In addition to the other answers, here's a simpler way to accomplish the same thing.

std::set<char> vs{'a', 'o', 'u', 'e', 'i'};
for (auto&& c : s1) {
    if (std::isdigit(c))
        if ((c - '0') % 2 == 0) // Trick to convert numeric char -> int.
            ++evens;
        else
            ++odds;
    else if (std::isalpha(c))
        if (vs.count(std::tolower(c)))
            ++vowels;
        else
            ++consonants;
}
Felix Glas
  • 15,065
  • 7
  • 53
  • 82
  • Thanks my friend i have already solved my problem but i still like diferent aproaches. Can you explain a bit what the code does for me and other begginers is it a bit dificult to foolow especially here for (auto&& c : s1) { and here if (std::find(std::begin(vs), std::end(vs), std::tolower(c)) != std::end(vs)) if (std::isdigit(c)) if ((c - '0') % 2 == 0) – Johnson Dec 11 '14 at 18:37
  • @Johnsonvdg `for (Type e : container)` is a *range-based for loop* and you can read about it [**here**](http://www.cprogramming.com/c++11/c++11-ranged-for-loop.html). [`std::find`](http://en.cppreference.com/w/cpp/algorithm/find) is an algorithm from the standard library that iterates over a range and returns an iterator if the specified value is present in the range. If you don't know how this works, then I recommend that you pick up a [good C++ book](http://stackoverflow.com/q/388242/873025) as explaining this is beyond the scope of this answer. – Felix Glas Dec 11 '14 at 21:31