C Program Returns Unexpected Numbers

Question

I am sorry if the question is actually dumb, but I just started learning C and no one I know was able to help. I am supposed to count the number of letters in the stdin and say how many times each one was used. It is also expected that we use a counter and the fread method.

EDIT: Igor helped me with his answer and the programm does now almost everything right. Small mistakes still remain: Only the first 10 chars are recognized and counted the others are ignored. And the input must be bigger than 10 or else it doesn't work.

I commented the changes I made in the code so others can follow my mistakes.

The code I have is:

#include <stdio.h>
#include <ctype.h>

int main(){
  char text[10];
  int timesUsed[26]; // I initialized the array but didn't set the values to 0.
  int timesUsed[26] = {0}; //this line corrected the issue with getting random numbers
  int textLength = 0;
  char *ptr;
  while(fread(&text,sizeof(char),10,stdin)){
    ptr = &text[0];    
    while(1){
      if(isalpha(*ptr)){
    textLength++;
    *ptr = tolower(*ptr); //the original was only tolower(*ptr); which is useless
    timesUsed[((char) *ptr) - 'a']++;
      }
      if(*ptr == '\n') goto end; //the original had here only one '='
      /**ptr = '\0';*/
      if(ptr == &text[10]) goto end; // on both
      ptr ++;
    }
  }
 end:
  printf("Number of letters:%20d\n",textLength);
  int i;
  for(i=0;i<26;i++){
    if(timesUsed[i]>0){
      char thechar = (char) 'a' + i;
      printf("The letter %c was used %5d time(s).\n",thechar,timesUsed[i]);
    }
  }
  return 0;
}

Which doesn't print the expected result, what it actually prints is:

Testing testing testing testing testing testing M
Number of letters:                   9
The letter e was used     2 time(s).
The letter g was used     1 time(s).
The letter i was used     1 time(s).
The letter n was used     1 time(s).
The letter s was used     1 time(s).
The letter t was used     3 time(s).

The changes made the output a lot more meaningful. Still some mistakes though.

Answer 1

Well, let's try to improve up some things:

#include <stdio.h>
#include <ctype.h>

int main(){
  char text[10] = ""; // This is allocating space for exactly 10 chars.
  int timesUsed[26] = {0}; // Always initialize your variables. Particularly arrays.
  int textLength = 0;
  int inputLength = 0;
  inputLength = fread(&text,sizeof(char),10,stdin); // fread returns the number of chars actually read, and since you're telling it to read 10 chars, that's the MAXIMUM amount of chars it'll read. It may read less, that's why it returns to you how much it read; 
  for(int i=0;i<inputLength;i++){ // since you know the length, iterate over it!
      if(isalpha(text[i])){
        textLength++;
        char c = tolower(text[i]); // No need to mess with pointers!
        timesUsed[text[i] - 'a']++;
      }
      if(text[i] == '\n') break; //Exit the for loop. NEVER EVER EVER USE GOTO!!! Actually I don't think this will ever happen because I think fread returns when the user hits ENTER 
  }
  printf("Number of letters:%20d\n",textLength);
  int i;
  for(i=0;i<26;i++){
    if(timesUsed[i]>0){
      char thechar = (char) 'a' + i;
      printf("The letter %c was used %5d time(s).\n",thechar,timesUsed[i]);
    }
  }
  return 0;

It will probably compile fine, but I didn't check. If not, it's probably some typo. Hope it helps.

Answer 2

That code seems awfully complicated to me. Here's an all-pointer arithmetic version. No array notation whatsoever:

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <ctype.h>
#include <limits.h>

#define A ((unsigned char)'a')
#define Z ((unsigned char)'z')

#define BUFFER_SIZE ((size_t)8192)

int main( int argc, char *argv[] )
{
  int            counter_max = 1 + UCHAR_MAX ;
  int           *counter     = calloc( (size_t) counter_max , sizeof(int)     ) ;
  unsigned char *buffer      = calloc( BUFFER_SIZE          , sizeof(*buffer) ) ;
  int            bufl        ;

  while ( (bufl=fread(buffer,sizeof(*buffer),BUFFER_SIZE,stdin)) > 0 )
  {
    for ( unsigned char *p = buffer ; p < buffer+bufl ; ++p )
    {
        unsigned char c = tolower(*p) ;
        ++ *(counter+c) ;
    }
  }

  for ( int *p = counter+A ; p <= counter+Z ; ++p )
  {
    int  n = *p ;
    if ( n > 0 )
    {
      char c = p-counter ;
      printf( "The letter '%c' was found %d times.\n" , c , n  ) ;
    }
  }

  return 0;
}

Answer 3

try this :

#include <stdio.h>
int main(){
  const char *str = "this is string to count";
  int counts[256] = { 0 };
  int i,letters=0;
  size_t len = strlen(str);
  for (i = 0; i < len; i++) {
    counts[(int)(str[i])]++;
  }

  for (i = 0; i < 256; i++) {
    if(counts[i]>0){
      printf("%c occurs %d times.\n", i , counts[i]);
      if((char)i!=' '){
        letters+=counts[i];
      }
    }

  }
 printf("Number of letters : %d", letters);
 getchar();
    return 0;

}

output :

  occurs 4 times.
c occurs 1 times.
g occurs 1 times.
h occurs 1 times.
i occurs 3 times.
n occurs 2 times.
o occurs 2 times.
r occurs 1 times.
s occurs 3 times.
t occurs 4 times.
u occurs 1 times.
Number of letters : 19

Answer 4

Next time compile your code with -Wall and -Wextra to notice some mistakes by yourself.

You have to initialize the array:

int timesUsed[26] = {0};

When you don't do that it contains random values and the count is not correct.

This statement has no effect:

tolower(*ptr);

Do this instead:

*ptr = tolower(*ptr);

And in C = is not the same as == which checks for equality, so you should change the if statements to:

 if(*ptr == '\n') goto end;
      /**ptr = '\0';*/
 if(ptr == &text[10]) goto end;