-1

Here is my code: the function that gives me errors is count_sentences.

int count_letters(string text);
int count_words(string text);
int count_sentences(string text);

int main(void)
{
    string text = get_string("Text: ");
    printf("%s\n", text);

    int letters = count_letters(text);
    printf("%i letters\n", letters);

    int words = count_words(text);
    printf("%i words\n", words);

    int sentences = count_sentences(text);
    printf("%i sentences\n", sentences);
}

int count_letters(string text)
{
    int letters = 0;

    for (int i = 0, len = strlen(text); i < len; i++)
    {
        if (isalpha(text[i]))
        {
            letters++;
        }
    }
    return letters;
}

int count_words(string text)
{
    int words = 0;

    for (int i = 0, len = strlen(text); i < len; i++)
    {
        if (text[i] == ' ')
        {
            words++;
        }
    }
    return words + 1;
}

int count_sentences(string text)
{
    int sentences = 0;

    for (int i = 0, len = strlen(text); i < len; i++)
    {
        if (text[i] == '.' || '!' || '?')
        {
            sentences++;
        }
    }
    return sentences;
}

Its giving me:

readability.c:66:28: error: use of logical '||' with constant operand [-Werror,-Wconstant- 
logical-operand]
        if (text[i] == '.' || '!' || '?')
                           ^  ~~~
readability.c:66:28: note: use '|' for a bitwise operation
        if (text[i] == '.' || '!' || '?')
                           ^~
                           |
fatal error: too many errors emitted, stopping now [-ferror-limit=]

Anyone know how to fix it? I know I can just make an else if block and get the same result but I would like to know if I could do it this way. it seems more effective and looks a lot nicer.

stackoverflow is giving me an error that is mostly code so ignore this please

chqrlie
  • 131,814
  • 10
  • 121
  • 189
Boriscodes
  • 21
  • 2
  • The condition `text[i] == '.' || '!' || '?'` is really the same as `(text[i] == '.') || '!' || '?'` which is not what you want. I would assume that CS50 (which I guess you're using) would have had that information. – Some programmer dude Aug 10 '23 at 11:49
  • 1
    Said differently the syntactically correct way is `if ((text[i] == '.') || (text[i] == '!)' || (text[i] == '?')) ...` – Serge Ballesta Aug 10 '23 at 11:53
  • This is a FAQ but the best duplicate I found is about relational operators and a bit far-fetched. Does anyone know of a better duplicate for this? – Lundin Aug 10 '23 at 12:52
  • In addition to the options given, it may be a golden opportunity to learn about `switch` statements too. – Rogue Aug 10 '23 at 13:03

2 Answers2

3
if (text[i] == '.' || '!' || '?')

has the same meaning as

if (true)

That's because text[i] == '.' evaluates to either true or false, but both '!' and '?' evaluate to true, since their numeric value is not zero.

The correct test would be

if (text[i] == '.' || text[i] == '!' || text[i] == '?')

or simpler, check the whole string with strpbrk which returns a pointer to the found char or NULL if none of the characters to search for is found:

unsigned count_sentences(const char* text) {
    unsigned sentences = 0;
    for (; *text; ++text) {
        text = strpbrk(text, ".!?"); // search for any of '.', '!', '?'
        if (text == NULL) break;     // if none of the chars was found
        ++sentences;
    }
    return sentences;
}

Demo

Ted Lyngmo
  • 93,841
  • 5
  • 60
  • 108
2

The || operator compares two booleans and stops evaluating them as soon as one is true. In your case, text[i] == '.' || '!' || '?', this means:

  1. evaluate text[i] == '.'. If true, stop and return true. Otherwise,
  2. evaluate '!'. This is a constant character. Because it is not 0, it is always true. That's not the logic you want to express and the compiler detects that it is weird to test a constant variable. This is a warning because the program could compile like that. Good job on activating -Werror to detect this early!

What you want to do is keep comparing text[i] to other constants like that: text[i] == '.' || text[i] == '!' || text[i] == '?'.

Since this is getting quite verbose and you might want to add other special characters later, you will want to use a more condensed syntax. Refer to this question for that: In C - check if a char exists in a char array

deribaucourt
  • 424
  • 8