Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #72851 > unrolled thread

While loop with bool, same issue

Back to article view | Back to comp.lang.c

Contents

  While loop with bool, same issue Alla _ <modelling.data@gmail.com> - 2015-10-14 08:59 -0700
    Re: While loop with bool, same issue Eric Sosman <esosman@comcast-dot-net.invalid> - 2015-10-14 13:31 -0400
      Re: While loop with bool, same issue August Karlstrom <fusionfile@gmail.com> - 2015-10-14 20:11 +0200
        Re: While loop with bool, same issue Eric Sosman <esosman@comcast-dot-net.invalid> - 2015-10-14 14:26 -0400
      Re: While loop with bool, same issue Ben Bacarisse <ben.usenet@bsb.me.uk> - 2015-10-14 20:44 +0100
    Re: While loop with bool, same issue Siri Cruz <chine.bleu@yahoo.com> - 2015-10-14 10:45 -0700
      Re: While loop with bool, same issue Keith Thompson <kst-u@mib.org> - 2015-10-14 13:15 -0700
    Re: While loop with bool, same issue Keith Thompson <kst-u@mib.org> - 2015-10-14 11:42 -0700
      Re: While loop with bool, same issue Jorgen Grahn <grahn+nntp@snipabacken.se> - 2015-10-15 08:20 +0000

#72851 — While loop with bool, same issue

Hello!

I have decided to create a new topic, although
with the issue that I have already raised previously 
(previous one is clear to me now).

/** Counting words in a piece of text **/

#include <stdio.h>
#include <stdbool.h>

bool alphabetic (const char c)
{
    if ( (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') )
        return true;
    else
        return false;
}
void read_line (char buffer [])
{
    char character;
    int i = 0;
    
    do
    {
        character = getchar();
        buffer[i] = character;
        i++;
    }
    while (character != '\n');
    
    buffer[i - 1] = '\0'; // i - 1 because the index character was incremented one extra time inside the loop the last time it was executed
}
int count_words (const char string[])
{
    int i; 
    int word_count = 0;
    bool look_for_word = true;
    
    for (i = 0; string[i] != '\0'; i++)
        if (alphabetic (string [i]))
        {
            if (look_for_word)
            {
                word_count++;
                look_for_word = false;
            }
        }
    
        else
            look_for_word = true;
    
    return word_count;
}

int main (void)
{
    char text[81];
    int total_words = 0;
    bool end_of_text = false;
    
    printf ("Type in your text.\n");
    printf ("When done, press 'RETURN'.\n\n");
    
    while ( !end_of_text ) 
    {
        read_line (text);
        
        if (text[0] == '\0')
            end_of_text = true;
        /** tests the input line that is stored
        inside the text array to see if just the Enter (or Return) 
        key was pressed; if so, then the buffer contains the null 
        string, in which case the flag end_of_text is set to true
        to signal that all of the text has been entered **/
        else
            total_words += count_words (text);
    }
    
    printf ("\nThere are %i words in the above text.\n", total_words);
    
    return 0;
    
}

The part that I am not sure I understand correctly:

while ( !end_of_text ) 
    {
        read_line (text);
        
        if (text[0] == '\0')
            end_of_text = true;

        /** tests the input line that is stored
        inside the text array to see if just the Enter (or Return) 
        key was pressed; if so, then the buffer contains the null 
        string, in which case the flag end_of_text is set to true
        to signal that all of the text has been entered **/

        else
            total_words += count_words (text);
    }
    
The program has assigned a false value to end_of_text.

Hence, the condition in the while loop should mean
end_of_text is true. 

This is how I read each line of the code:

Before the user has typed in his string, end_of_text 
is true (no text exists); therefore, while (end_of_text is true),
get the text from the user (read_line(text).

If the user has only pressed Enter, set end_of_ text to true.
Isn't end_of_text already true because we are inside the loop with 
that exact condition; and also because read_line(text) ends 
with the NULL character, thus indicating the end of the text?

Otherwise, set end_of_text to false and compute total_words.

I am not sure I understand why while loop is required.

Thank you!

[toc] | [next] | [standalone]

#72861

On 10/14/2015 11:59 AM, Alla _ wrote:
> Hello!
>
> I have decided to create a new topic, although
> with the issue that I have already raised previously
> (previous one is clear to me now).
>
> /** Counting words in a piece of text **/
>
> #include <stdio.h>
> #include <stdbool.h>
>
> bool alphabetic (const char c)
> {
>      if ( (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') )
>          return true;
>      else
>          return false;
> }

     Aside: The `if' is unnecessary.  You could simply do

	return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');

instead.  (I mention this mostly because you seem to use this
silly pattern frequently.  It's not "wrong," just silly.)

> void read_line (char buffer [])
> {
>      char character;
>      int i = 0;
>
>      do
>      {
>          character = getchar();
>          buffer[i] = character;
>          i++;
>      }
>      while (character != '\n');
>
>      buffer[i - 1] = '\0'; // i - 1 because the index character was incremented one extra time inside the loop the last time it was executed
> }

     Aside: This is, more or less, a re-implementation of the
COMPLETELY UNSAFE gets() function, a function SO BAD that it has
been removed from the latest C Standard.  (I say "more or less"
because there are errors: See question 12.1 on the comp.lang.c
FAQ page at <http://www.c-faq.com/> for one of them, and you'll
learn about the other if you ever type your system's end-of-input
keystroke -- maybe CTRL-D or CTRL-Z -- at the keyboard.)

> int count_words (const char string[])
> {
>      int i;
>      int word_count = 0;
>      bool look_for_word = true;
>
>      for (i = 0; string[i] != '\0'; i++)
>          if (alphabetic (string [i]))
>          {
>              if (look_for_word)
>              {
>                  word_count++;
>                  look_for_word = false;
>              }
>          }
>
>          else
>              look_for_word = true;
>
>      return word_count;
> }
>
> int main (void)
> {
>      char text[81];
>      int total_words = 0;
>      bool end_of_text = false;
>
>      printf ("Type in your text.\n");
>      printf ("When done, press 'RETURN'.\n\n");
>
>      while ( !end_of_text )
>      {
>          read_line (text);
>
>          if (text[0] == '\0')
>              end_of_text = true;
>          /** tests the input line that is stored
>          inside the text array to see if just the Enter (or Return)
>          key was pressed; if so, then the buffer contains the null
>          string, in which case the flag end_of_text is set to true
>          to signal that all of the text has been entered **/
>          else
>              total_words += count_words (text);
>      }
>
>      printf ("\nThere are %i words in the above text.\n", total_words);
>
>      return 0;
>
> }
>
> The part that I am not sure I understand correctly:
>
> while ( !end_of_text )
>      {
>          read_line (text);
>
>          if (text[0] == '\0')
>              end_of_text = true;
>
>          /** tests the input line that is stored
>          inside the text array to see if just the Enter (or Return)
>          key was pressed; if so, then the buffer contains the null
>          string, in which case the flag end_of_text is set to true
>          to signal that all of the text has been entered **/
>
>          else
>              total_words += count_words (text);
>      }
>
> The program has assigned a false value to end_of_text.

     To start with, yes.  (Actually, it's an initialization, not an
assignment, but that's a nitpick.)

> Hence, the condition in the while loop should mean
> end_of_text is true.

     Did you miss the leading `!' character?  Have you learned
yet what the `!' means?

> This is how I read each line of the code:
>
> Before the user has typed in his string, end_of_text
> is true (no text exists); therefore, while (end_of_text is true),
> get the text from the user (read_line(text).

     No; see the remark about `!' above.

> If the user has only pressed Enter, set end_of_ text to true.
> Isn't end_of_text already true because we are inside the loop with
> that exact condition; and also because read_line(text) ends
> with the NULL character, thus indicating the end of the text?

     No.

> Otherwise, set end_of_text to false and compute total_words.

     The variable is initialized to false, but is never set false
thereafter.  The only value assigned to it after initialization
is true.

> I am not sure I understand why while loop is required.

     How many lines of input could you process if there were no loop?

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#72865

On 2015-10-14 19:31, Eric Sosman wrote:
>      Aside: The `if' is unnecessary.  You could simply do
>
>      return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');

Or maybe

	return isalpha(c);

after including ctype.h.


-- August

[toc] | [prev] | [next] | [standalone]

#72868

On 10/14/2015 2:11 PM, August Karlstrom wrote:
> On 2015-10-14 19:31, Eric Sosman wrote:
>>      Aside: The `if' is unnecessary.  You could simply do
>>
>>      return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');
>
> Or maybe
>
>      return isalpha(c);
>
> after including ctype.h.

     I think that's been mentioned elsethread.  Also, note that
you need

	return isalpha((unsigned char) c);

... thanks to some unfortunate "hysterical raisins."

-- 
esosman@comcast-dot-net.invalid
"Don't be afraid of work. Make work afraid of you." -- TLM

[toc] | [prev] | [next] | [standalone]

#72873

Just an FYI post...

Eric Sosman <esosman@comcast-dot-net.invalid> writes:

> On 10/14/2015 11:59 AM, Alla _ wrote:
>> Hello!
>>
>> I have decided to create a new topic, although
>> with the issue that I have already raised previously
>> (previous one is clear to me now).
>>
>> /** Counting words in a piece of text **/
>>
>> #include <stdio.h>
>> #include <stdbool.h>
>>
>> bool alphabetic (const char c)
>> {
>>      if ( (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') )
>>          return true;
>>      else
>>          return false;
>> }
>
>     Aside: The `if' is unnecessary.  You could simply do
>
> 	return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');
>
> instead.  (I mention this mostly because you seem to use this
> silly pattern frequently.  It's not "wrong," just silly.)

Sadly, it's from the book.

>> void read_line (char buffer [])
>> {
>>      char character;
>>      int i = 0;
>>
>>      do
>>      {
>>          character = getchar();
>>          buffer[i] = character;
>>          i++;
>>      }
>>      while (character != '\n');
>>
>>      buffer[i - 1] = '\0'; // i - 1 because the index character was incremented one extra time inside the loop the last time it was executed
>> }
>
>     Aside: This is, more or less, a re-implementation of the
> COMPLETELY UNSAFE gets() function, a function SO BAD that it has
> been removed from the latest C Standard.  (I say "more or less"
> because there are errors: See question 12.1 on the comp.lang.c
> FAQ page at <http://www.c-faq.com/> for one of them, and you'll
> learn about the other if you ever type your system's end-of-input
> keystroke -- maybe CTRL-D or CTRL-Z -- at the keyboard.)

Again, it's from the textbook though with a slight name change and a
comment added.

Dorothy Parker comes to mind...  "This is not a book to be cast aside
lightly.  It should be thrown with great force."  (paraphrased)

No book on C should be cavalier about buffer overruns.

<snip>
-- 
Ben.

[toc] | [prev] | [next] | [standalone]

#72862

In article <96769962-2192-444e-a738-01306756e949@googlegroups.com>,
 Alla _ <modelling.data@gmail.com> wrote:

> bool alphabetic (const char c)
> {
>     if ( (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') )
>         return true;
>     else
>         return false;
> }

(1) Letters are not guaranteed contiguous. In EBCDIC they are not.
(2) Don't be afraid of using the library.
(3) Eventually char may be a 16 bit integer carrying Unicode. Letters are not 
contiguous in Unicode.

#include <ctype.h>

bool alphabetic (const char c) {return isalpha(c);}


> void read_line (char buffer [])
> {
>     char character;
>     int i = 0;
>     
>     do
>     {
>         character = getchar();
>         buffer[i] = character;
>         i++;
>     }
>     while (character != '\n');
>     
>     buffer[i - 1] = '\0'; // i - 1 because the index character was 
>     incremented one extra time inside the loop the last time it was executed
> }

(1) If you passed in the buffer size, you could check for buffer overrun.
(2) and you could use fgets.
(3) Pretend end of file is an empty line so the function client doesn't have to 
deal with that.

void read_line (char buffer [], size_t n) {
    if (n==0) {
        ;
    }else if (fgets(buffer, n, stdin)) {
        char *nl = strchr(buffer, '\n');
        if (nl) *nl = 0;
    }else {
        *buffer = 0
    }
}

> int count_words (const char string[])
> {
>     int i; 
>     int word_count = 0;
>     bool look_for_word = true;
>     
>     for (i = 0; string[i] != '\0'; i++)
>         if (alphabetic (string [i]))
>         {
>             if (look_for_word)
>             {
>                 word_count++;
>                 look_for_word = false;
>             }
>         }
>     
>         else
>             look_for_word = true;
>     
>     return word_count;
> }
> 
> int main (void)
> {
>     char text[81];
>     int total_words = 0;
>     bool end_of_text = false;
>     
>     printf ("Type in your text.\n");
>     printf ("When done, press 'RETURN'.\n\n");
>     
>     while ( !end_of_text ) 
>     {
>         read_line (text);
>         
>         if (text[0] == '\0')
>             end_of_text = true;
>         /** tests the input line that is stored
>         inside the text array to see if just the Enter (or Return) 
>         key was pressed; if so, then the buffer contains the null 
>         string, in which case the flag end_of_text is set to true
>         to signal that all of the text has been entered **/
>         else
>             total_words += count_words (text);
>     }
>     
>     printf ("\nThere are %i words in the above text.\n", total_words);
>     
>     return 0;
>     
> }
> 
> The part that I am not sure I understand correctly:
> 
> while ( !end_of_text ) 
>     {
>         read_line (text);
>         
>         if (text[0] == '\0')
>             end_of_text = true;
> 
>         /** tests the input line that is stored
>         inside the text array to see if just the Enter (or Return) 
>         key was pressed; if so, then the buffer contains the null 
>         string, in which case the flag end_of_text is set to true
>         to signal that all of the text has been entered **/
> 
>         else
>             total_words += count_words (text);
>     }
>     
> The program has assigned a false value to end_of_text.
> 
> Hence, the condition in the while loop should mean
> end_of_text is true. 

On the first iteration end_of_text==false, !end_of_text==true, the loop 
condition is true, and the iteration continues.

> This is how I read each line of the code:

! means not. The loop condition is while end_of_text is false.

> I am not sure I understand why while loop is required.

If you want to read more than one line, you need some kind of iteration.

-- 
:-<> Siri Seal of Disavowal #000-001. Disavowed. Denied. Deleted.
'I desire mercy, not sacrifice.'
When is a Kenyan not a Kenyan? When he's a Canadian.
That's People's Commissioner Siri Cruz now. Punch!

[toc] | [prev] | [next] | [standalone]

#72875

Siri Cruz <chine.bleu@yahoo.com> writes:
[...]
> (3) Eventually char may be a 16 bit integer carrying Unicode.

16 bits is not enough to represent all Unicode characters.

>  Letters are not contiguous in Unicode.

No, but a-z and A-Z are.  Depending on the program's requirements, you
might actually want to check whether a character is one of the 52 Latin
letters, rather than whether it's a "letter" in any more general sense.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#72871

Alla _ <modelling.data@gmail.com> writes:
> I have decided to create a new topic, although
> with the issue that I have already raised previously 
> (previous one is clear to me now).
>
> /** Counting words in a piece of text **/
>
> #include <stdio.h>
> #include <stdbool.h>
>
> bool alphabetic (const char c)
> {
>     if ( (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') )
>         return true;
>     else
>         return false;
> }

This is more simply written as:

    return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');

Strictly speaking the parentheses aren't necessary, but they do make it
clearer.

There's not much point in defining the parameter as "const".  It has no
meaning to the caller.

> void read_line (char buffer [])
> {
>     char character;
>     int i = 0;
>     
>     do
>     {
>         character = getchar();

getchar() returns an int, but you're assigning it to a char.  That means
you can't reliably detect EOF ...

>         buffer[i] = character;
>         i++;
>     }
>     while (character != '\n');

... which you're not trying to do anyway.  It *is* possible for
getchar() to return EOF in the middle of a line.  If it does, you have
an infinite loop.

>     buffer[i - 1] = '\0'; // i - 1 because the index character was incremented one extra time inside the loop the last time it was executed
> }
> int count_words (const char string[])
> {
>     int i; 
>     int word_count = 0;
>     bool look_for_word = true;

I suggest that "looking_for_word" would be a clearer name.

>     for (i = 0; string[i] != '\0'; i++)
>         if (alphabetic (string [i]))
>         {
>             if (look_for_word)
>             {
>                 word_count++;
>                 look_for_word = false;
>             }
>         }
>     
>         else
>             look_for_word = true;
>     
>     return word_count;
> }
>
> int main (void)
> {
>     char text[81];
>     int total_words = 0;
>     bool end_of_text = false;
>     
>     printf ("Type in your text.\n");
>     printf ("When done, press 'RETURN'.\n\n");
>     
>     while ( !end_of_text ) 
>     {
>         read_line (text);

`text` is just 81 characters long.  Your read_line function will read as
many characters as you give it.  If your cat stands on the space bar,
you'll have undefined behavior.

[snip]

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <http://www.ghoti.net/~kst>
Working, but not speaking, for JetHead Development, Inc.
"We must do something.  This is something.  Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"

[toc] | [prev] | [next] | [standalone]

#72929

On Wed, 2015-10-14, Keith Thompson wrote:
> Alla _ <modelling.data@gmail.com> writes:
...
>> bool alphabetic (const char c)
>> {
>>     if ( (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') )
>>         return true;
>>     else
>>         return false;
>> }

> There's not much point in defining the parameter as "const".  It has no
> meaning to the caller.

For the benefit of the OP, a clarification:

Not much point in /this/ function, but if it had been more complex
the const could help detect errors where you modify 'c' by accident.

/Jorgen

-- 
  // Jorgen Grahn <grahn@  Oo  o.   .     .
\X/     snipabacken.se>   O  o   .

[toc] | [prev] | [standalone]

Back to top | Article view | comp.lang.c

csiph-web