scanf does not work with &str when str is defined as char str[100]

Please, can someone tell me what is the problem in my syntax. I want to find the duplicate letters in a word. it is working properly if I declare a character array here itself but not working with scanf.

#include<stdio.h>

// Finding the duplicate alphabets in a string

int length(char str[]) //Finding the length of the string
{
    int len;
    while(str[len]!='\0')
    {
        len++;
    }
    return len;
}
void duplicate(char str[],int n)
{
    int i,j,flag;
    for(i=0;i<=n-2;i++) //Selecting the alphabet for comparison
    {
        flag=0;
        if(str[i]!='\0')
        {
            for(j=i+1;j<=n-1;j++) //comparison of alphabets
            {
                if(str[j]==str[i])
                {
                    flag=1;
                    str[j]=0;
                }
            }
            if(flag==1)
            {
                printf("%c is the duplicate character\n",str[i]);
            }   
            
        }        
        
    } 
}
int main()
{
    char str[100];
    scanf("%s",&str);
    int n= length(str);
    duplicate(str,n);
}

>Solution :

The problems that I noticed:

  1. main: scanf("%s",&str); is the wrong type for str (char (*)[100]) and should be scanf("%s", str);. Also, scanf is not safe to use, so consider using fgets instead. char str[100] uses a magic 100 value, instead #define STR_LEN 100 so you can do char str[STR_LEN].
  2. length: int len; is uninitialized and should be int len = 0; (len is not a great variable name as it’s usually 1 bigger than last index, but you use it to index with). Why did you write your own instead of using strlen?
  3. duplicate (minor issue): it’s good practice to minimize variable scope, so declare i and j in the lops (for(int i=0; ...), and flag inside the first for loop where you assign flag=0. I think you just refactor it to:
void duplicate(char str[],int n) {
    for(int i=0;i<=n-2 && str[i];i++) {
        for(int j=i+1;j<=n-1;j++) {
            if(str[i] == str[j]) {
                printf("%c is the duplicate character\n", str[i]);
                str[j] = 0;
            }
        }

    }
}

The str[j] = 0; is a bit iffy, it removed the 2nd instance of the character. If you leave it in aaa will report 2 duplicates and if you remove it it will report 3. The way the message is phrased, it sounds like it should only be reported once per character. To do that you have have to replace the duplicate values with a sentinal (that you otherwise ignore) and it can’t be 0, or you add each duplicate to an array and only report it a duplicate if it’s not in that array (i.e. emulating a set).

You can also create an array of counts for each letter. Initialized to 0, and add 1 as you find each letter. Then report the letters with count > 1:

#define CHARS 256

void duplicate(char str[],int n) {
    int counts[CHARS] = { 0 };
    for(unsigned i=0;i<=n-1 && str[i];i++) {
        counts[(unsigned) str[i]]++;
    }
    for(unsigned i=0; i<CHARS; i++) {
        if(counts[i] > 1) {
            printf("%c is the duplicate character\n", i);
        }
    }
}

Leave a Reply