# First non-repeated character in a string in c

Write an efficient function to find the first non-repeated character in a string. For example, the first non-repeated character in "total" is 'o' and the first non-repeated character in "teeter" is 'r'.

Please see my solution and give me some feedback and suggestions for improving and optimizing it if needed.

#include <stdio.h>
#define SIZE 100                /* max size of input string */
#define LIM 5                   /* max number of inputs */
char repeat_ch(const char *);   /* return first non-repeated character */
int main(void)
{
char line[SIZE];
int i = 0;
char ch;

while(i < LIM && gets(line))
{
ch = repeat_ch(line);
if(ch != NULL)
printf("1st non-repeated character: %c\n", ch);
else
printf("There is no unique character in a string: %s\n", line);
i++;
}

}

char repeat_ch(const char *string)
{
char array[130] = {0};
char *p = string;

/* store each character in array, use ascii code as an index for character
* increment each time the same character appears in a string
*/
while(*p)                               // stop when '\0' encountered
{
array[*p]+=1;
p++;
}
while(*string)
{
if(array[*string] == 1)
return *string;         // stop when unique character found
string++;
}

return *string;
}

-

Alex, it looks quite efficient. There are some issues that I see:

• gets should never be used. Use fgets (but note that you will have to strip the trailing \n)

• define main at the end to avoid the need for a prototype for repeat_ch

• declare repeat_ch as static

• your limit does not work as i is not incremented. But why not stop on reading an empty string rather than limit the number of loops?

• NULL is normally defined as (void *) 0 so comparing a char with NULL is wrong. The compiler will warn you of that. Just use 0 or better '\0'

In repeat_ch

• function name is inaccurate - function looks for a non-repeated char.

• the two // comments are noisy (ie. don't tell reader anything)

• array would be better sized 256

• p should be const

• for-loops would be better

for (const char *p=string; *p; ++p) {
array[*p] += 1;
}
for (const char *p=string; *p; ++p) {
if (array[*p] == 1) {
return *p;
}
}
return '\0';

-
thanks for feedback was really useful. Have added "i" as it was my copy-paste typo. – alex Dec 31 '12 at 15:20
+1: Very good analysis. The only things I can see that you didn't mention are trivia: (1) erratic indentation in main() and (2) use of if(...) etc instead of if (...). The omitted return 0; from main() is OK in C99 or later, though personally I think that rule was a mistake and always include it. – Jonathan Leffler Jan 1 '13 at 7:16