1

My program should keep prompting the user to enter the correct format of string if there are duplicates of letters or the string contains non-alphabetic characters.

However, it does not work correctly. It seems that it cannot find out whether characters are alphabetic or not, while works fine to distinguish duplicate letters and keep prompting the user to enter string one more time.

Also, I've tried to do it without isalpha() function. Instead of that, I checked for condition of uppercase character belonging into the range between 'A' and 'Z'. Even in that case the program did not work right.

Here is the function and test that I've tried:

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

int check(char key[]) {
   int flag = 1;
   int counts[strlen(key)];

   int i, j;

   for (i = 0; i < strlen(key); i++) {
       counts[i] = 0;
   }

   for (i = 0; i < strlen(key); i++) {
       for (j = 0; j < strlen(key); j++) {
           if (toupper(key[i]) == toupper(key[j])) {
               counts[i]++;
           }
       }
   }

   for (i = 0; i < strlen(key); i++) {
       if (counts[i] > 1 || isalpha(key[i]) == 0) {
           /* if I remove 
              isalpha related condition it works fine 
              to find only duplicate letters
              and keep asking user to input 
              another string */
           flag = 0;
       }
   }
   return flag;
}

int main() {
   int size = 20;
   char str[size];

   printf("Enter a string w/o duplicate letters and non-alphabetic characters: ");
   fgets(str, size, stdin);

   //test 1. str = "hello" -> prompt to enter str again since there 2 'l's (this part works well but only to find duplicate letters)
   //test 2. str = "helo12" -> prompt to enter str again since there are non-alphabetic characters
   //test 3. str = "naruto" -> program keeps working further
   while (check(str) != 1) {
       printf("Illegal word! \nEnter again: ");
       fgets(str, size, stdin);
   }
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
sunset.
  • 71
  • 1
  • 1
  • 8
  • 1
    Edit to include a sample input series, The response series you are seeing, and the expected series you wnat to see. (i.e. _series_ in this context means character series.) – ryyker Apr 13 '20 at 18:13
  • 4
    A line read in with `fgets` retains the non-alphabetic new-line character, so what you see might be `"ambidextrous\n"`. Remove it before you process the string. ([Here's how](https://stackoverflow.com/a/28462221/2979617).) – M Oehm Apr 13 '20 at 18:15
  • Your check function is doing too much. Once you know you have a failure, stop checking. eg `int check(char *key){ int counts[256] = {0}; for( ; *key && *key != '\n'; key++) { if(counts[toupper(*key)]++ == 1 || ! isalpha(*key)) { return 1; } } return 0; }` – William Pursell Apr 13 '20 at 18:33
  • the function: `strlen()` returns a value with type `size_t`, NOT `int`. Suggest your loop counters: `i` and `j` be declared as `size_t`, not `int` – user3629249 Apr 15 '20 at 01:44
  • when compiling, always enable the warnings, then fix those warnings. ( for `gcc`, at a minimum use: `-Wall -Wextra -Wconversion -pedantic -std=gnu11` ) Note: other compilers use different options to produce the same results. – user3629249 Apr 15 '20 at 01:45
  • @ryyker, Not an `int` variable, rather a `size_t` variable – user3629249 Apr 15 '20 at 01:48
  • OT: in general, it is best to limit the scope of variables as much as reasonable. Therefore, the `for()` statements (which currently are: `for (i = 0; i < strlen(key); i++) {` would be much better written as: `for ( size_t i = 0; i < strlen(key); i++) {` – user3629249 Apr 15 '20 at 01:50
  • OT regarding: `int size = 20;` This would be much better placed just after the `#include` statements and written as: `#define SIZE 20` Then use: `SIZE` everywhere that `size` is currently being used. – user3629249 Apr 15 '20 at 01:53
  • regarding: `for (i = 0; i < strlen(key); i++) { for (j = 0; j < strlen(key); j++) { if (toupper(key[i]) == toupper(key[j])) { counts[i]++; } } }` This will increment `counts` once for every time the indexes `i` and `j` are the same (besides every time there is a duplicate letter. Not what you want to do. – user3629249 Apr 15 '20 at 02:00
  • regarding: `fgets(str, size, stdin);` There are many things a user could do to cause this to fail, including entering a EOF Therefore, always check the returned value (not the contents of `str`) to assure the user has not messed up the input operation – user3629249 Apr 15 '20 at 02:03

4 Answers4

3

The check() function should work as expected but you do not strip the trailing newline left in str by fgets(), causing the check() to fail.

Note also that isalpha() and toupper() are undefined for negative values, which is the case of non-ASCII bytes on platforms where char is signed by default. Use isalpha((unsigned char)key[i]) and toupper((unsigned char)key[i]) to avoid this problem.

Here is a simpler method:

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

int check(const char *key) {
    while (*key) {
        unsigned char c = *key++;
        if (!isalpha(c))
            return 0;
        if (strchr(key, c) || strchr(key, tolower(c)) || strchr(key, toupper(c)))
            return 0;
    }
    return 0;
}

int main() {
   char str[20];

   printf("Enter a string without duplicate letters or non-alphabetic characters: ");
   if (!fgets(str, sizeof str, stdin))
       return 1;
   str[strcspn(str, "\n")] = '\0'; // strip the trailing newline if any

   // test 1. str = "hello"  -> prompt to enter str again since there are 2 'l's
   // test 2. str = "helo12" -> prompt to enter str again since there are non-alphabetic characters
   // test 3. str = "naruto" -> program keeps working further
   while (check(str) != 1) {
       printf("Illegal word!\nEnter again: ");
       if (fgets(str, sizeof size, stdin) == NULL)
           return 1;
       str[strcspn(str, "\n")] = '\0'; // strip the trailing newline if any
   }
   printf("String is valid: %s\n", str);
   return 0;
}
chqrlie
  • 131,814
  • 10
  • 121
  • 189
2

Another simple approach rather than multiple comparisons is to check against an accepted character set with strspn and then check for duplicates. In your case you could define the accepted characters as the alpha characters and then use a simple frequency array to check for duplicates, e.g.

#define ACCEPT "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"

int check (char *s)
{
    int freq['Z'-'A'+1] = {0};

    if (s[strspn(s, ACCEPT)])           /* validate all alpha-char */
        return 0;

    for (int i = 0; s[i]; i++)          /* check for dups */
        if (++freq[toupper(s[i]) -'A'] > 1)
            return 0;

    return 1;
}

(note: the characters are converted to upper-case for the frequency comparison)

In the first case if the "length" of the string is not equal to the number of acceptable characters in the string, you have an unwanted character. If all characters are acceptable, strspn returns the length leaving s[length] as the nul-character (or just 0) The next loop just loops over each character and if any appear more than once, the string fails the test.

A short example incorporating the test could be:

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

#define MAXC 256
#define ACCEPT "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"

int check (char *s)
{
    int freq['Z'-'A'+1] = {0};

    if (s[strspn(s, ACCEPT)])           /* validate all alpha-char */
        return 0;

    for (int i = 0; s[i]; i++)          /* check for dups */
        if (++freq[toupper(s[i]) -'A'] > 1)
            return 0;

    return 1;
}

int main (void) {

    char str[MAXC];

    for (;;) {  /* loop continually until valid input received */
        fputs ("enter string, alpha-only w/o duplicates: ", stdout);
        if (!fgets (str, MAXC, stdin)) {        /* validate read */
            puts ("(user canceled input)");     /* handle EOF */
            return 1;
        }
        str[strcspn (str, "\n")] = 0;           /* remove '\n' */
        if (check (str))                        /* if good str, break */
            break;
        fputs ("error: invalid entry\n\n", stderr);
    }

    printf ("\ngood str: %s\n", str);           /* output result */
}

Another way to skin-the-cat in C.

David C. Rankin
  • 81,885
  • 6
  • 58
  • 85
0

you need to stop in strlen(key)-1, and return flag=0 as soon as you find a number

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

int check(char key[]) {
   int flag = 1;
   int counts[strlen(key)];

   int i, j;

   for (i = 0; i < strlen(key); i++) {
       counts[i] = 0;
   }

   for (i = 0; i < strlen(key); i++) {
       for (j = 0; j < strlen(key); j++) {
           if (toupper(key[i]) == toupper(key[j])) {
               counts[i]++;
           }
       }
   }

   for (i = 0; i < strlen(key)-1; i++) {
       if (counts[i] > 1 || isalpha(key[i])==0 ) {
           /* if I remove
              isalpha related condition it works fine
              to find only duplicate letters
              and keep asking user to input
              another string */
           return flag = 0;
       }
   }
   return flag;
}

int main() {
   int size = 20;
   char str[size];

   printf("Enter a string w/o duplicate letters and non-alphabetic characters: ");
   fgets(str, size, stdin);

   //test 1. str = "hello" -> prompt to enter str again since there 2 'l's (this part works well but only to find duplicate letters)
   //test 2. str = "helo12" -> prompt to enter str again since there are non-alphabetic characters
   //test 3. str = "naruto" -> program keeps working further
   while (check(str) != 1) {
       printf("Illegal word! \nEnter again: ");
       fgets(str, size, stdin);
   }
}
Masmoudi
  • 123
  • 1
  • 9
  • Stopping at `strlen(str)-1` will prevent the detection of a non-alpha character at the end of the string. – chqrlie Apr 13 '20 at 18:58
  • no it is done to stop it from checking \n and giving wrong output. try it it works perfectly – Masmoudi Apr 13 '20 at 21:29
  • Not stripping the newline **is** the error in the posted code. Bending the semantics of the `check()` function is not the proper way to fix the problem. As a matter of fact, try entering `toto!`Ctrl-D and you will see that `toto!` will be accepted with the trailing `!` because it does not have a trailing newline. Furthermore, you will see that the program goes into an infinite loop because end-of-file is not properly tested either. – chqrlie Apr 13 '20 at 21:47
  • I tried toto! and it was not accepted. Also i could not find information about ! not having a newline after it in google. – Masmoudi Apr 14 '20 at 01:22
  • you did not signal the end of file after the `!`. If the last line input has no newline, the last character is not tested, hence `toto!` would be accepted if followed immediately by the end of file. – chqrlie Apr 14 '20 at 09:02
0

Your code is working well. You just have to remove trailing newline character from fgets() input.

#include <stdio.h>
#include <string.h>
#include <ctype.h>
 int check(char key[]) {
  int flag = 1;
  int counts[strlen(key)];

    int i, j;

   for (i = 0; i < strlen(key); i++) {
     counts[i] = 0;
    }

   for (i = 0; i < strlen(key); i++) {
     for (j = 0; j < strlen(key); j++) {
        if (toupper(key[i]) == toupper(key[j])) {
           counts[i]++;
       }
   }
 }

  for (i = 0; i < strlen(key); i++) {
      if (counts[i] > 1 || isalpha(key[i]) == 0) {
        /* if I remove 
          isalpha related condition it works fine 
          to find only duplicate letters
          and keep asking user to input 
          another string */
       flag = 0;
   }
 }
  return flag;
 }


 int main() {
    int size = 20;
    char str[size];

    printf("Enter a string w/o duplicate letters and non-alphabetic characters: ");
    fgets(str, size, stdin);
   str[strcspn(str, "\r\n")] = 0;

         //test 1. str = "hello" -> prompt to enter str again since there 2 'l's (this part works well but only to find duplicate letters)
         //test 2. str = "helo12" -> prompt to enter str again since there are non-alphabetic characters
       //test 3. str = "naruto" -> program keeps working further
      while (check(str) != 1) {
         printf("Illegal word! \nEnter again: ");
         fgets(str, size, stdin);
          str[strcspn(str, "\r\n")] = 0;
        }
       }
Avinash Kumar Singh
  • 447
  • 4
  • 14