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:
- main:
scanf("%s",&str);
is the wrong type forstr
(char (*)[100]
) and should bescanf("%s", str);
. Also,scanf
is not safe to use, so consider usingfgets
instead.char str[100]
uses a magic 100 value, instead#define STR_LEN 100
so you can dochar str[STR_LEN]
. - length:
int len;
is uninitialized and should beint 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 usingstrlen
? - 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 assignflag=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);
}
}
}