0

I'm stuck with this exercise. It suppose to read a text[30] and return the number of words that contains but always returns (int) 1 instead of the right value. When trying to debug it, I added a printf call as a last statement into each "branch" of the if statement, (which is contained in a while loop), in the countWords function. Compiling and executing it after this makes the program to return the correct number of words. I'm pretty sure there should be something wrong in the code but I can't find the mistake. If not, is printf affecting it in any way? Any suggestion about whole the code is welcome. Regards.

int main (void)
{
    void readText (char [], const int);
    int countWords (char []);
    void printResult (int);

    const int upBound = 30;
    char string[upBound];
    int result;

    printf ("Write a text and press 'return' an extra time when done\n\n");
    readText (string, upBound);
    result = countWords (string);
    printf ("\nThe text is %d word(s) long.", result);

    return 0;
}

The next two functions read the text.

int readLine (char line[], const int upBound)
{
    static int i = 0;
    char tmp;

    do
    {
        tmp = getchar ();
        if (i < (upBound - 1))
        {
            line[i++] = tmp;
        }
    }
    while ((tmp != '\n'));
    return i;
}

void readText (char fString[], const int upBound)
{
    int readLine (char [], const int);
    int i;

    while ((fString[(i = readLine (fString, upBound)) - 2] != '\n')
      && (i < (upBound - 1)));
    if (i == (upBound - 1)) fString [(upBound - 1)] = '\0';
    else fString[--i] = '\0';
}

The last two functions should count the words and test whether characters are alphabetic or white spaces respectively.

int countWords (char fString[])
{
    bool testAlphabetic (char);

    int i = 0, counter = 0;
    bool lfw = true;

    while (fString[i] != '\0')
    {
        if ((lfw) && (testAlphabetic (fString[i])))
        {
            ++counter;
            lfw = false;
            ++i;
            printf ("1");  // This is the test
        }
        else if (!(testAlphabetic (fString[i])))
        {
            lfw = true;
            ++i;
            printf ("2");  // This is the test
        }
        else
        {
            ++i;
            printf ("3");  // This is the test
        }
    }

    return counter;
}

bool testAlphabetic (char character)
{
    bool isAlphabetic;

    if (((character >= 'a') && (character <= 'z')) || ((character >= 'A') && (character <= 'Z')))
    {
        isAlphabetic = true;
    }

    return isAlphabetic;
}
Sebastian Redl
  • 69,373
  • 8
  • 123
  • 157
Gabriel
  • 13
  • 4
  • 2
    Is this supposed to be C or C++? –  Jan 15 '14 at 12:40
  • 1
    When adding printf solves mysterious problems I suspect corruption of the stack due to some piece of code writing outside of its memory. Also, why are you repeating your functions *inside* the functions that use them? – fvu Jan 15 '14 at 12:42
  • Sometimes `printf` can slow down the execution and make it works. But it doesn't seems to be this here. – jmlemetayer Jan 15 '14 at 12:43
  • As I'm supposing this is howework I'm not going to give ready-to-paste answers: consider `static int i = 0;` in `readLine()` - what does the static do there, specifically what will happen on the second execution of `readLine()`? – fvu Jan 15 '14 at 12:44
  • Passing a character array as `char []` instead of `char *` might actually send it by value. In this case the `readText` function will not fill the initial array - it will fill its copy instead. – aragaer Jan 15 '14 at 12:47
  • 1
    @aragaer No, `char []` for a function parameter means the exact same thing as `char *` (both in C and in C++). –  Jan 15 '14 at 12:48
  • @fvu I think this is done on purpose. `i` is used as a global counter associated with the input buffer declared in `main`. –  Jan 15 '14 at 12:50
  • @hvd My bad, gotta sleep more. It would be true for function, but not for array. – aragaer Jan 15 '14 at 12:52
  • @fvu: It's not my homework don't worry. readLine() is supposed to read just a line up to the next \n. The meaning of static int i = 0; is to not being initialized each time it's called by readText(). In that way It's sure It won't go up exceeding the size of the array (const int upBound), no matter how many times is called by readText(). – Gabriel Jan 15 '14 at 15:21
  • @aragaer: I understand what you mean but I don't know where you want to mean that. – Gabriel Jan 15 '14 at 15:22

1 Answers1

4
bool testAlphabetic (char character)
{
    bool isAlphabetic;

    if (((character >= 'a') && (character <= 'z')) || ((character >= 'A') && (character <= 'Z')))
    {
        isAlphabetic = true;
    }

    return isAlphabetic;
}

You do not initialize isAlphabetic here. If isAlphabetic = true; is not executed, then its value is indeterminate.

You should replace bool isAlphabetic; with bool isAlphabetic = false;. Actually the variable is not needed as you can also write:

bool testAlphabetic (char character)
{
    return ((character >= 'a') && (character <= 'z'))
        || ((character >= 'A') && (character <= 'Z'));
}

Also there is a standard library function isalpha in ctype.h doing basically the same.

  • And I would add that if your compiler doesn't warn you about that, you're using it wrong. – Gilles 'SO- stop being evil' Jan 15 '14 at 13:11
  • 1
    @Gilles gcc 4.8.1 actually doesn't warn me about this, even with `-Wall -Wextra -pedantic`. It seems to loose track of the initialization state as soon as the if-statement is non-trivial. –  Jan 15 '14 at 14:23
  • @Nabla IIRC, that's a long-standing missing feature: by the time the compiler gets to where it checks for uninitialised values, the value is no longer uninitialised, because an earlier optimisation pass already noticed that `testAlphabetic` cannot ever return anything other than `true`, so replaced it with a simple `return true;`. (Except when optimisations are disabled, in which case the detection of uninitialised variables doesn't work anyway.) –  Jan 15 '14 at 15:17
  • Wow, I didn't know about that case. Nice to know, thanks. @hvd [This SO thread](http://stackoverflow.com/questions/17705880/gcc-failing-to-warn-of-uninitialized-variable) links to a bug report, and there's also a [wiki page](http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings). – Gilles 'SO- stop being evil' Jan 15 '14 at 15:30
  • @hvd It didn't work with isAlphabetic = true; either. However, it seems to work fine having the function to return true or false. Could you explain me why please? – Gabriel Jan 15 '14 at 15:31
  • @hvd I'm sorry, I still don't understand why it finally works. – Gabriel Jan 15 '14 at 15:40
  • @user3197717 I don't understand your question. I'm not saying you *should* unconditionally return true. For a function where the whole point is to test whether a character is a letter, it should certainly return `false` if the character is not a letter. What you have in the question just makes sure it returns `true` if the character is a letter, but you haven't given it any specific behaviour for other characters. –  Jan 15 '14 at 15:40
  • @hvd I understand what you want to mean now! Basically I didn't define the function to return false in case the if condition isn't satisfied. Thank you very much!! – Gabriel Jan 15 '14 at 16:23