0

So I have to create a program that reads the user input and shows how many times each letter appears in that string, and also how many non-letters but my code for alphabets is showing random numbers..

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

#define SIZE 100

void readInput (char string[]);
void Calc(char string[], int letters[]);

void
readInput (char string[])
{
    printf ("Enter a String:\n");
    fgets (string, SIZE, stdin);

    string[strlen (string) - 1] = '\0';
}

void
Calc(char string[], int letters[])
{
    int c = 0, x;
    while (string[c] != '\0')
    {
        if (string[c] >= 'a' && string[c] <= 'z')
        {
            x = string[c] - 'a';
            letters[x]++;
        }

        c++;
    }

    for (c = 0; c < 26; c++)
        printf ("%c occurs %d times in the entered string.\n", c + 'a', letters[c]);
}

int
main ()
{
    char string[SIZE];
    int letters[26];

    readInput (string);
    Calc(string, letters);
    return 0;
}

This is the output

I'm new to strings I've googled examples but can't seem to find whats wrong with my code and no idea how I will include the non-letters part.

WedaPashi
  • 3,561
  • 26
  • 42
Mohammed
  • 31
  • 5
  • Is `string[strlen (string) - 1] = '\0';` for chopping off the linefeed? – unwind Nov 22 '17 at 10:33
  • Referring to "how does one make the non letters part?" from @Mohammed below: by using `isalpha`. If that returns 0 the character is a non-letter, else it is a letter. Or to make it easier to derive the array index, you can use `isupper` and `islower`. – Weather Vane Nov 22 '17 at 10:41

3 Answers3

4

The contents of letters are not initialised. Formally the behaviour of your program is indeterminate.

Sort that by writing int letters[26] = {0}; Doing that sets all elements to zero, which is what you want in this case.

Bathsheba
  • 231,907
  • 34
  • 361
  • 483
4

letters[] is uniinitialized. Solution int letters[26]={0}.

You are reading an uninitialized value which is indeterminate. And the results doesn't conform to as you expect it to be.

Here you want to initialize the elements with 0 denoting that you haven't seen any charcaters yet. (automatic storage duration).

A better way to overwrite the '\n' would be

string[strcspn(string, "\n")] = 0;
user2736738
  • 30,591
  • 5
  • 42
  • 56
  • Not undefined. Just indeterminate. –  Nov 22 '17 at 10:25
  • 1
    `You are reading an uninitialized value which is undefined behavior.`..that's an overstatement. – Sourav Ghosh Nov 22 '17 at 10:26
  • `letters` is passed to a function, so it has it's address taken. Therefore, reading it isn't undefined behavior, even if it's uninitialized. –  Nov 22 '17 at 10:26
  • 1
    Guys.... the values them self are indeterminate. *Accessing* the values is technically speaking: **undefined behaviour** – MrPaulch Nov 22 '17 at 10:27
  • 1
    @MrPaulch no. It's only undefined behavior if the object being read could have been declared `register`. –  Nov 22 '17 at 10:28
  • Just answered this on a similar topic, see there for reference: https://stackoverflow.com/a/47431348/2371524 –  Nov 22 '17 at 10:30
  • 1
    @FelixPalmen that is a good discussion, it boils down to just about any non-aggregate type with automatic storage could have been declared register... The only bit I take issue with is one of the final comments "writing a single member is enough to have an initialized object". It's not writing to a single member that does it, it is *initializing* a single member that causes all remaining members to be treated as if they had been declared with static storage duration (and thereby initialized to zero). – David C. Rankin Nov 22 '17 at 10:39
  • @DavidC.Rankin could you move your comment over there? I'd have some questions about it, but it's clearly off-topic here ;) –  Nov 22 '17 at 10:43
  • Sure, glad to I'll give the standard cites as well. – David C. Rankin Nov 22 '17 at 10:43
0

you should initialize letters with all 0 values

Adrian
  • 21
  • 5