6
\$\begingroup\$

Background

Just got started with C and now am testing a few pages and simple problems to get the syntax right.

On Programiz they provide a few very easy problems with solutions.

This program checks if user input is a Vowel or a Consonant or none of those. I solve slightly different, but would be glad to see criticism to details I may not even be aware of.

Environment

  • For editing the code, I use Vim without any pluggins.
  • For testing code I am using Google's picoc which is a runtime much like NodeJS or Python's where -in interactive mode- you type C code and it is evaluated right away, so you don't need compilation.

Code

The code is commented to reflect what I understand of each step. Double starred comments will produce documentation. Then I mostly use /* syntax */.

/* importing modules */
#include <stdio.h>/** standard C library for input output.*/
#include <stdbool.h>/** bool data type. Not sure if this is normally used.*/
#include <ctype.h>/** character types, isalpha() function.*/

/** main takes no input, sends no output.*/
void main(void){

  /* set up */
  bool isVowel; char letter; char vowels[] = "aeiouAEIOU";

  /* take user input */
  printf("Insert a character: ");
  scanf("%c", &letter);//Store input at the address of letter.

  /* first check this is an alphabetic character */
  if(!isalpha(letter)) {printf("Not an alphabetic character\n"); }

  int nOfVowels = sizeof(vowels)/sizeof(vowels[0]);

  for(int a=0; a<nOfVowels; a++){

    if(vowels[a]==letter){ isVowel=true; }

  }

  printf("%s\n", isVowel == false ? "Consonant": "Vowel");

}
\$\endgroup\$

3 Answers 3

7
\$\begingroup\$

The minimum set of flags you should use with gcc/clang is -Wall -Wextra -pedantic and fix all warnings:

$ gcc -Wall -Wextra -pedantic main.c
main.c:6:6: warning: return type of ‘main’ is not ‘int’ [-Wmain]
    6 | void main(void){ // main takes no input, sends no output.
      |      ^~~~

You said

/** importing modules */

In C world we call this header files, not modules like in other languages such as Python.

You check if input is an alphabetic character

if(!isalpha(letter)) {printf("Not an alphabetic character\n"); }

but you carry on checking if it's a consonant or vowel which doesn't make sense:

$ ./a.out
Insert a character:    m
Not an alphabetic character
Consonant

I think you should exit with error here:

#include <stdlib.h>
(...)
  if(!isalpha(letter))
    {
      printf("Not an alphabetic character\n");
      return EXIT_FAILURE;
    }

You can check exit status in your shell:

$ ./a.out
Insert a character:      m
Not an alphabetic character
$ echo $?
1

Formatting is a matter of taste but I'd prefer variable definitions like this

bool isVowel; char letter; char vowels[] = "aeiouAEIOU";

to be divided into several lines:

bool isVowel;
char letter;
char vowels[] = "aeiouAEIOU";

You can leave for(int a=0; a<nOfVowels; a++) loop right after finding the first character that is a vowel:

  for(int a=0; a<nOfVowels; a++)
    {
      if(vowels[a] == letter)
        {
          isVowel = true;
          break;
        }
    }

Not much of a difference performance-wise when there are only 10 elements in vowels array but you could see bigger gains if larger array was used.

\$\endgroup\$
2
  • \$\begingroup\$ Thank you so much. What would you define EXIT_FAILURE to? Also if you could explain the first line that'd be great. \$\endgroup\$ Commented Feb 9, 2022 at 10:23
  • 4
    \$\begingroup\$ You don't have to define it on your own, it's defined in platform-specific manner in stdlib.h. All gcc options are described in man gcc \$\endgroup\$ Commented Feb 9, 2022 at 10:29
5
\$\begingroup\$

The only portable return type for main() is int - although this one function is "magic" in that it doesn't need a return statement. Unlike other functions, we're allowed to just run off the end and the compiler will infer a success status for the program.

We should always check input operations, as even reading a single character may fail (e.g. if we run ./a.out </dev/null, reading will return the EOF value). In this case, we need scanf() to report exactly 1 successful conversion to be sure that letter is assigned a value:

printf("Insert a character: ");
fflush(stdout);

if (scanf("%c", &letter) != 1) {
    fputs("Input error\n", stderr);
    return EXIT_FAILURE;      /* defined in <stdlib.h> */
}

There's a classic gotcha with the use of isalpha(). It requires an unsigned character value, but char can be a signed type (e.g. in ISO-8859.1, accented letters can have negative values). The fix for this is quite simple in this program: just declare letter as unsigned char instead, and everything else can remain unchanged.

When we search vowels, we can simplify a bit. We know that sizeof vowels[0] must be 1 because it's a char, and that's the unit of sizeof. Also, we can stop searching (using break) as soon as we find a match:

for (unsigned a = 0;  a < sizeof vowels;  ++a) {
    if (vowels[a] == letter) {
        isVowel = true;
        break;
    }
}

I'd go a bit further, and make this into a function we can call from main():

static bool isVowel(unsigned char c)
{
    static const char vowels[] = "aeiouAEIOU";
    for (unsigned a = 0;  a < sizeof vowels;  ++a) {
        if (c == vowels[a]) {
            return true;
        }
    }
    return false;
}

This function can be further simplified by using the library function strchr() (in <string.h>) which returns a non-null (true) pointer if the character is found and a null (false) one if not:

static bool is_vowel(int c)
{
    static const char vowels[] = "aeiouAEIOU";
    return strchr(vowels, c);
}

When we use the result, note that isVowel == false is the same as !isVowel - or even better, we can swap the order of the conditional to use the positive sense:

isVowel ? "Vowel" : "Consonant"

Modified code

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

static bool is_vowel(int c)
{
    static const char vowels[] = "aeiouAEIOU";
    return strchr(vowels, c);
}

int main(void)
{
    printf("Insert a character: ");
    fflush(stdout);

    unsigned char letter;
    if (scanf("%c", &letter) != 1) {
        fputs("Input error\n", stderr);
        return EXIT_FAILURE;
    }

    if (isalpha(letter)) {
        const bool is_v = is_vowel(letter);
        printf("%d, %s\n",  is_v,  is_v ? "Vowel" : "Consonant");
    } else {
        printf("Not an alphabetic character\n");
    }
}

A final note on internationalisation. Although this is a simple beginner project, it can help demonstrate the problems we have when code interacts with the Real World. For example, in the word "hymn", y acts as a vowel, but the program doesn't understand this. It also won't recognise vowels such as à or ŵ. When writing real programs, it's important to get clear definitions as part of your requirements.

Don't let that complication put you off learning more C, though!

\$\endgroup\$
5
  • \$\begingroup\$ In static const char vowels[], what benefit does static provide? \$\endgroup\$ Commented May 27, 2023 at 14:55
  • 1
    \$\begingroup\$ It saves on stack and on array initialisation (in a future version, where we call the function more than once) if the compiler isn't smart enough to treat it the same as const char *const vowels = "aeiouAEIOU"; \$\endgroup\$ Commented May 27, 2023 at 15:39
  • 1
    \$\begingroup\$ Bug: bool is_vowel('\0') returns true. strchr(vowels, c) iterates up to 11 times. for (unsigned a = 0; a < sizeof vowels; ++a) { better as for (unsigned a = 0; vowels[a]; ++a) {. Note: sizeof vowels is 11. \$\endgroup\$ Commented May 30, 2023 at 1:27
  • 1
    \$\begingroup\$ With unsigned char letter;, consider bool is_vowel(unsigned c) for maximal portability. \$\endgroup\$ Commented May 30, 2023 at 1:32
  • 1
    \$\begingroup\$ I'll add the usual constraint that anything else is non-portable, @chux. \$\endgroup\$ Commented Jun 6, 2023 at 7:07
2
\$\begingroup\$

Bug: Code considers '\0' is a vowel

char vowels[] = "aeiouAEIOU"; is an array of size 11, not 10. for(int a=0; a<nOfVowels; a++){ iterates up to 11 times.

With scanf("%c", &letter);, it is possible, although tricky, to enter the null character for letter.

Suggest instead:

// for(int a=0; a<nOfVowels; a++){
for(int a=0; vowels[a]; a++){
\$\endgroup\$

You must log in to answer this question.