Array debugging C?

Question

An assignment in the K&R C Programming 2nd edition says that I have to write a program to print a histogram of the lengths of words in its input. I believe I know how to do so, but when running a test program on arrays, which I have only just learned about, all I get is "8", no matter what I input. This is the program so far:

#include <stdio.h>

/* write a program to print a histogram
of the lengths of words in its input */
main()
{
      int wl[11];
      int cc, c;

      while ((c=getchar()) != EOF);
      {
            if (c != ' ')
               ++cc;
            if (c == ' ' && cc == '1')
            {
               ++wl[0];
               c = 0;
            }
       putchar(wl[0]);
      }
}

It may be just because I'm a total beginner in programming, but I honestly cannot see where I went wrong here. Any help would be appreciated.

In addition to the other answers, remove the semicolon at the end of the while statement; otherwise, the while will read all the input before moving on to the next line. — Mark Tolonen
– Mark Tolonen, Commented Jul 8, 2012 at 1:31
Thanks to everyone's help, I have now successfully got the program working. Thank you all. — user1509423
– user1509423, Commented Jul 8, 2012 at 2:15

D.Shawley · Accepted Answer · 2012-07-08 01:04:35Z

2

Start by initializing your variables:

int wl[11] = {0};
int cc = 0;

By default, memory contains garbage in C.

EDIT: Beyond the Obvious

the comparison cc == '1' does not do what you expect. It should probably be cc == 1
my guess is that ++wl[0] should be ++wl[cc]. This assumes a maximum word length of 11.
c = 0 should be resetting cc instead - c is the current character, cc is the current word length
putchar needs a character not an int so you will need to solve that as well

Good start though.

edited Jul 8, 2012 at 1:04

answered Jul 8, 2012 at 0:58

D.Shawley

59.9k10 gold badges105 silver badges116 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

D.Shawley Over a year ago

Yup. printf is what you want here. You probably want the printf outside of the while as well.

Jerry Coffin · Accepted Answer · 2012-07-08 01:06:46Z

1

You have a number of problems here.

main()

Though it was fairly common to omit the return type at that time, it isn't any more. You probably want to write this as:

int main()

Instead.

{
      int wl[11];

As @D.Shawley already pointed out, this will contain garbage.

      int cc, c;

What he forgot to point out was that these will too. This doesn't matter much for c, but does for cc.

      while ((c=getchar()) != EOF);
      {
            if (c != ' ')
               ++cc;

Here you're incrementing cc, but you never set it to a known value first, so you have no idea what it started as, or (therefore) what result your increment will produce.

            if (c == ' ' && cc == '1')

I'm guessing you don't really want '1' here -- you probably just want 1. That is, you don't want to compare cc to the digit '1', but to the value 1.

            {
               ++wl[0];
               c = 0;

Though it's harmless, zeroing c doesn't accomplish much here -- maybe you wanted to zero cc? I'm not sure what you're really trying to do though. At a guess, you probably don't want to increment wl[0] either.

            }
       putchar(wl[0]);

You probably don't want putchar here either -- you probably want to write out wl[0] as an int or (to produce a histogram) use it as the count of something like '*' to print out.

answered Jul 8, 2012 at 1:06

Jerry Coffin

494k83 gold badges656 silver badges1.2k bronze badges

10 Comments

Jerry Coffin Over a year ago

@KeithThompson: What's the benefit for a function you never call anyway? Or are you honestly recommending a recursive main (outside of the IOCCC anyway)?

user1509423 Over a year ago

Why wouldn't I want to increment wl[0]? If I didn't, I wouldn't be able to get the program to do what I want. Perhaps I'm not seeing something you are.

Jerry Coffin Over a year ago

@user1509423: Presumably you want to count up the number of letters in a word, then increment the int in the array that corresponds to that length. When you print them out, you probably want to print out more than wl[0] as well.

Josh Petitt Over a year ago

@Keith int main(int argc, char *argv[]); is probably more correct for a system with STDIN and STDOUT ;-)

Keith Thompson Over a year ago

@JerryCoffin: The C standard mentions int main(void) and int main(int argc, char *argv[]), or equivalent forms. Any other forms are permitted as implementation-defined extensions, but are not guaranteed to be accepted. int main() is not quite equivalent to int main(void). And int main() is an old-style function definition; IMHO it's best to avoid those.

|

Josh Petitt · Accepted Answer · 2012-07-08 01:55:47Z

I'd use argc and argv here (look this up in your C book). The nice thing about this is it gives you the number of words and you don't have to handle input yourself (let the OS and your standard library handle it). You could malloc the sizes array (the histogram buckets), but you could also make a "reasonable" assumption that will get the majority of English words (hopefully you aren't German :-) ).

#include <string.h>
#include <stdio.h>

// assume longest word is "Supercalifragilisticexpialidocious"    
#define MAX_WORD_LENGTH    (35)

int main(int argc, char* argv[])
{
  size_t sizes[MAX_WORD_LENGTH] = {0};

  for (int i=0; i<argc; ++i)
  {
    size_t len = strlen(argv[i]);

    if (len < MAX_WORD_LENGTH)
    {
      ++sizes[len]; // increment bucket
    }
    else
    {
      ++sizes[0]; // too long, put in default bucket
    }
  }

  // print the buckets
  for (int i=1; i<MAX_WORD_LENGTH; ++i)
  {
    printf("%d ", sizes[i]); 
  }

  // print the default bucket
  printf("%d", sizes[0]);
}

Collectives™ on Stack Overflow

Array debugging C?

3 Answers 3

1 Comment

10 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

1 Comment

10 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related