I'd like to know what is the issue which cause wrong display in my code?

Question

I'd like to know what's the problem where i want to Write a function which, given a string, return TRUE if all characters are distinct and FALSE if any character is repeated.


#include <stdio.h>
char distinct_or_not_string(char *string []);
int main()
{ 
  char str[20];
  char result;
  printf("please entre your string:");
  gets(str);
  printf("\n");
 result=distinct_or_not_string(str);   

}
char distinct_or_not_string(char *string [])
{
    int i=0,j;
    while(string[i]!='\0')
    {
        j=i+1;
        while(string[i]!='\0')
        {
              if(string[i]==string[j])
              {
                  return printf("false");
              }
         j++;
        }
        i++;
    }
    return printf("true");
}

The second while loop condition is using wrong index. Should be `j` instead of `i` — kaylum, Mar 15 '20 at 09:00
[Why is the gets function so dangerous that it should not be used?](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used) — RobertS supports Monica Cellio, Mar 15 '20 at 09:44

srilakshmikanthanp · Answer 1 · 2020-03-15T12:04:02.453

0

change this

 char distinct_or_not_string(char *string [])

to

 char distinct_or_not_string(char string [])

and

change this in second while loop

while(string[i]!='\0')

to

while(string[j]!='\0')

edited Mar 15 '20 at 12:04

answered Mar 15 '20 at 09:22

srilakshmikanthanp

2,231
1
8
25

RobertS supports Monica Cellio · Accepted Answer · 2020-03-16T08:07:31.240

Your code has four issues.

You use gets to catch the string from stdin. Never ever use gets. It is deprecated because of security reasons since with that, you could easily run into buffer overflow, when the input string is longer than the buffer itself. gets don´t stop scanning.

Why is the gets function so dangerous that it should not be used?

Use fgets instead, which limits the input according to the size of the buffer:

fgets(str, sizeof str, stdin);

Wrong parameter type of distinct_or_not_string():

char distinct_or_not_string(char *string []);

string is of type char ** but you want to pass a pointer to a string. Use either char string[] or char *string where string is of type char *.

The second while loop uses the the wrong object to proof the condition, i instead of j:

while(string[i]!='\0')

Use of printf as return value of distinct_or_not_string():

return printf("false");

printf() will be always evaluated to 1 (assuming no error has occurred at printing) and thus result in the main function would always have 1 regardless of the string has any character twice or more or not, which is not appropriate.

Split both and make each statement separate:

printf("false");
return 0;

and

printf("true");
return 1;

Side note: distinct_or_not_string() does not need to return a char; the return value in this case has nothing to do with a string or a character of it.

So the return type of distinct_or_not_string() shall, in my opinion, preferable be int for better readability and to avoid confusions, although it isn´t a syntactic mistake to use char instead. The declaration of the function would then be like that:

int distinct_or_not_string(char *string);

The corrected code should be like that:

#include <stdio.h>

int distinct_or_not_string(char* string);

int main()
{ 
  char str[20];
  int result;

  printf("please enter your string: ");
  char *p = fgets(str,sizeof str,stdin);
  if(p == NULL)
  {
      printf("Error occurred at reading the input!");
      return 1;
  }

  printf("\n");

  result = distinct_or_not_string(str); 
  return 0;  
}

int distinct_or_not_string(char *string)
{
    int i=0,j;
    while(string[i] != '\0')
    {
        j=i+1;
        while(string[j] != '\0')
        {
              if(string[i] == string[j])
              {
                  printf("false");
                  return 0;
              }
         j++;
        }
        i++;
    }
    printf("true");
    return 1;

If you want to test this code online, here is the link: https://onlinegdb.com/H1PCstoSL

but in the fourth issue (i want to return the statement which tell user true or false so i never mind what is the actual results of return either 0 nor 1 @roberts-supports-monica-cellio — sara hamdy, Mar 15 '20 at 10:25
@sarahamdy I added a bit more, maybe this will help you to understand how it works - if every character is distinct from eah other you use `printf("true")` and return the value 1 and if not, you use `printf("false")` and return the value 0. — RobertS supports Monica Cellio, Mar 15 '20 at 10:28
@sarahamdy You can test the code here: https://onlinegdb.com/SJ_tNKiHU — RobertS supports Monica Cellio, Mar 15 '20 at 10:34
so you put return 0 , return 1 to distinct between the two returns i mean in my code the return will be 1 in both (either distinct or not) as i used printf statement in the return statement which always returns 1 so to be more accurate we shouldn't use printf statment in return — sara hamdy, Mar 15 '20 at 10:34
@sarahamdy The problem is when you want to know if every character was distinct or not inside the `main()` function for use it later. Of course, You print out that it is true or false but **if** you would want to use the result of `distinct_or_not_string` anywhere else, you need to know if it was so. `result` in `main` is meant for that and it needs to have the appropriate value in it **if** you want to make anything else in the program based off if the string has distinct characters or not. — RobertS supports Monica Cellio, Mar 15 '20 at 10:38
@sarahamdy But if you do not want to do something else later - the program has only this task - you could omit the return value and the `return` object in `main` at all and use the return type of `void`: `void distinct_or_not_string(char *string)` - the type `void` means this function has no valid return value and do not need to have one. — RobertS supports Monica Cellio, Mar 15 '20 at 10:42
@sarahamdy Here is the link to the Wikipedia site about type `void` and what a function with `void` return value is. I recommend you to take a look at this page. https://en.wikipedia.org/wiki/Void_type — RobertS supports Monica Cellio, Mar 15 '20 at 10:46

score 0 · Answer 3 · answered Mar 15 '20 at 10:11

my mistake is in the second loop where i used the same index i instead of j so the condition is always true so the following code works successfully

#include <stdio.h>
char distinct_or_not_string(char string []);
int main()
{ 
  char str[20];
  char result;
  printf("please entre your string:");
  gets(str);
  printf("\n");
 result=distinct_or_not_string(str);   

}
char distinct_or_not_string(char string [])
{
    int i=0,j;
    while(string[i]!='\0')
    {
        j=i+1;
        while(string[j]!='\0')
        {
              if(string[i]==string[j])
              {
                  return printf("false");
              }
         j++;
        }
        i++;
    }
    return printf("true");
}

But that is not the one and only mistake you made. Look at my [answer](https://stackoverflow.com/a/60691594/12139179). — RobertS supports Monica Cellio, Mar 15 '20 at 10:15

I'd like to know what is the issue which cause wrong display in my code?

3 Answers3