Finding first recurring letter in each string of a given set

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
6
down vote

favorite
1












First input is the number of strings ex: 5

Second input is a set of strings ex: hello world how are you



Output the letter in the string ex: 'l' in string 1



Below is my code:



#include <stdio.h>
#include <stdlib.h>

void find_first_recurring_letter(char **str,int r,int c);

int main()

int r,i;
printf("Enter the number of strings to be entered");
scanf("%d",&r);
/*dynamic allocation for str*/
char **str=(char**)malloc(r*sizeof(char*));
for(i=0;i<r;i++)
str[i]=(char*)malloc(50*sizeof(char));
for(i=0;i<r;i++)

printf("Enter the string %d",i+1);
gets(str[i]);

for(i=0;i<r;i++)
printf("%sn",str[i]);
find_first_recurring_letter(str,r,50);
for(i=0;i<r;i++)
free(str[i]);
return 0;



void find_first_recurring_letter(char **str,int r,int c)

char *ptr,*sptr,ch[r];
int indx[r];
int i;
for(i=0;i<r;i++)

indx[i]=0;
ch[i]='';
for(ptr=&str[i][0];*ptr!='';ptr++)

for(sptr=&ptr[1];*sptr!='';sptr++)

if(*ptr==*sptr)
ch[i]= *ptr;
indx[i]=1;



for(i=0;i<r;i++)

if(ch[i]!='')
printf("'%c' in string %dn",ch[i],indx[i]+1);









share|improve this question





















  • You should fix This warnings/Errors
    – Michi
    Feb 7 at 18:55
















up vote
6
down vote

favorite
1












First input is the number of strings ex: 5

Second input is a set of strings ex: hello world how are you



Output the letter in the string ex: 'l' in string 1



Below is my code:



#include <stdio.h>
#include <stdlib.h>

void find_first_recurring_letter(char **str,int r,int c);

int main()

int r,i;
printf("Enter the number of strings to be entered");
scanf("%d",&r);
/*dynamic allocation for str*/
char **str=(char**)malloc(r*sizeof(char*));
for(i=0;i<r;i++)
str[i]=(char*)malloc(50*sizeof(char));
for(i=0;i<r;i++)

printf("Enter the string %d",i+1);
gets(str[i]);

for(i=0;i<r;i++)
printf("%sn",str[i]);
find_first_recurring_letter(str,r,50);
for(i=0;i<r;i++)
free(str[i]);
return 0;



void find_first_recurring_letter(char **str,int r,int c)

char *ptr,*sptr,ch[r];
int indx[r];
int i;
for(i=0;i<r;i++)

indx[i]=0;
ch[i]='';
for(ptr=&str[i][0];*ptr!='';ptr++)

for(sptr=&ptr[1];*sptr!='';sptr++)

if(*ptr==*sptr)
ch[i]= *ptr;
indx[i]=1;



for(i=0;i<r;i++)

if(ch[i]!='')
printf("'%c' in string %dn",ch[i],indx[i]+1);









share|improve this question





















  • You should fix This warnings/Errors
    – Michi
    Feb 7 at 18:55












up vote
6
down vote

favorite
1









up vote
6
down vote

favorite
1






1





First input is the number of strings ex: 5

Second input is a set of strings ex: hello world how are you



Output the letter in the string ex: 'l' in string 1



Below is my code:



#include <stdio.h>
#include <stdlib.h>

void find_first_recurring_letter(char **str,int r,int c);

int main()

int r,i;
printf("Enter the number of strings to be entered");
scanf("%d",&r);
/*dynamic allocation for str*/
char **str=(char**)malloc(r*sizeof(char*));
for(i=0;i<r;i++)
str[i]=(char*)malloc(50*sizeof(char));
for(i=0;i<r;i++)

printf("Enter the string %d",i+1);
gets(str[i]);

for(i=0;i<r;i++)
printf("%sn",str[i]);
find_first_recurring_letter(str,r,50);
for(i=0;i<r;i++)
free(str[i]);
return 0;



void find_first_recurring_letter(char **str,int r,int c)

char *ptr,*sptr,ch[r];
int indx[r];
int i;
for(i=0;i<r;i++)

indx[i]=0;
ch[i]='';
for(ptr=&str[i][0];*ptr!='';ptr++)

for(sptr=&ptr[1];*sptr!='';sptr++)

if(*ptr==*sptr)
ch[i]= *ptr;
indx[i]=1;



for(i=0;i<r;i++)

if(ch[i]!='')
printf("'%c' in string %dn",ch[i],indx[i]+1);









share|improve this question













First input is the number of strings ex: 5

Second input is a set of strings ex: hello world how are you



Output the letter in the string ex: 'l' in string 1



Below is my code:



#include <stdio.h>
#include <stdlib.h>

void find_first_recurring_letter(char **str,int r,int c);

int main()

int r,i;
printf("Enter the number of strings to be entered");
scanf("%d",&r);
/*dynamic allocation for str*/
char **str=(char**)malloc(r*sizeof(char*));
for(i=0;i<r;i++)
str[i]=(char*)malloc(50*sizeof(char));
for(i=0;i<r;i++)

printf("Enter the string %d",i+1);
gets(str[i]);

for(i=0;i<r;i++)
printf("%sn",str[i]);
find_first_recurring_letter(str,r,50);
for(i=0;i<r;i++)
free(str[i]);
return 0;



void find_first_recurring_letter(char **str,int r,int c)

char *ptr,*sptr,ch[r];
int indx[r];
int i;
for(i=0;i<r;i++)

indx[i]=0;
ch[i]='';
for(ptr=&str[i][0];*ptr!='';ptr++)

for(sptr=&ptr[1];*sptr!='';sptr++)

if(*ptr==*sptr)
ch[i]= *ptr;
indx[i]=1;



for(i=0;i<r;i++)

if(ch[i]!='')
printf("'%c' in string %dn",ch[i],indx[i]+1);











share|improve this question












share|improve this question




share|improve this question








edited Feb 7 at 9:25
























asked Feb 7 at 9:16









katty

1805




1805











  • You should fix This warnings/Errors
    – Michi
    Feb 7 at 18:55
















  • You should fix This warnings/Errors
    – Michi
    Feb 7 at 18:55















You should fix This warnings/Errors
– Michi
Feb 7 at 18:55




You should fix This warnings/Errors
– Michi
Feb 7 at 18:55










2 Answers
2






active

oldest

votes

















up vote
10
down vote



accepted










Better names



The variable names are too short to be meaningful. It's common to use i as an index in for-loops, but r doesn't really tell us its purpose. Names like number_of_strings are self-descriptive, so we should use those.



Unused variables



We never use c in find_first_recurring_letter, therefore we should remove the parameter altogether.



Strictly speaking, we also never use indx[i]. If ch[i] is '',indx[i]will be1(see "always use braces" below), and1` otherwise, so we can replace



 printf("'%c' in string %dn",ch[i],indx[i]+1);


by



 printf("'%c' in string %dn", ch[i], 2);


without changing the programs logic.



Simpler pointer logic



Instead of ptr = &str[i][0], which is ptr = &(*(str[i])) use ptr = str[i]. Instead of sptr = &ptr[1], just use sptr = ptr.



Again, we could call them current and next or similar.



By the way, all indx[i] are 1 (see "unused variables" above and "always use braces" below). That doesn't sound right. Also keep in mind that variable length arrays are optional from C11 on.



Get rid of additional memory



If we printf the recurring character, we can get rid of ch and indx:



 if(*ptr==*sptr)
printf("....");
indx[i]=1;


Always use braces



Note that the code above contains a bug. indx[1] will be set to 1 regardless whether *ptr == *sptr. The indentation is misleading here.



You probably added indx[i] later and forgot the add the braces, so we should always use braces to make sure that bugs like this cannot happen.



If you don't want to use braces, use a code formatter. The indentation on the automatically formatted code will hint possible errors.



Prefer late C99 declarations if possible



That way we can keep the scope of our variables short, e.g.



for(int i = 0; i < number_of_strings; ++i) 
for(char *current = str[i]; *current != ''; current++)
for(char *next = current + 1; *next != ''; next++)
if(*current == *next)
printf("'%c' in string %dn", *current, i);






It's now impossible to use next outside of its loop. Also, we should try to initialize our variables whenever possible.



Use const type* for inputs that are not supposed to change



That way we cannot accidentally modify your input values, e.g.



str[i][k] = ''


would not compile if str was const char**.



Check malloc's return



We should really check whether malloc returns NULL.



Split functions



find_first_recurring_letter doesn't follow its name completely. We find the first recurring letter on a set of strings on a per-string basis. That sounds perfect for a split:



const char * find_first_recurring_letter(const char * haystack) 
for(const char * current = haystack; *current != ''; current++)
for(const char * next = current + 1; *next != ''; next++)
if(*current == *next)
return current;



return NULL;


void find_first_recurring_letters(const char ** strings, int number_of_strings)
for(int i = 0; i < number_of_strings; i++)
const char * result = find_first_recurring_letter(strings[i]);

if(result)
printf("'%c' in string %dn", *result, i);





We can now use find_first_recurring_letter on any null-terminated string. We're able to reuse functionality. Note that both functions are very easy to check by hand.






share|improve this answer























  • but is there a better way of solving this problem @Zeta
    – katty
    Feb 7 at 11:59






  • 1




    @katty there are other algorithms with $mathcal O(n log k)$ time and $mathcal O(k)$ additional space (where k is the number of distinct characters), and a radix-like one with $mathcal O(n)$ time and $mathcal O(2 ^textsizeof(char))$ space. But those are other algorithms.
    – Zeta
    Feb 7 at 12:14


















up vote
3
down vote













@Zeta's answer is very good but I'd go ahead and improve the code by creating a few functions to improve readability:



void allocMemoryForStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
arrayOfStrings[i] = malloc(sizeof(*arrayOfStrings[i]) * 50);



void printStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
printf("%sn", arrayOfStrings[i]);



void freeStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
free(arrayOfStrings[i]);




P.S.: The code might need some tweaks to compile/work. Use it as a reference.






share|improve this answer



















  • 1




    "some tweaks"? You didn't rename str or r in your second and third example, and i wasn't declared at all.
    – Zeta
    Feb 7 at 16:19










  • @Zeta you're absolutely right. I've corrected that and also changed the function names to cameCase.
    – luizfzs
    Feb 7 at 18:07






  • 1




    arrayOfStrings[i] = (char*) malloc(50 * sizeof(char)); ---> cast not needed. Also, consider using the size of the reference type and not an explicit type.: arrayOfStrings[i] = malloc(sizeof *arrayOfStrings[i] * 50);. Easy to code. review and maintain.
    – chux
    Feb 7 at 19:25










  • @luizfzs Curious, do you find sizeof(*arrayOfStrings[i]) easier to read than without parens: sizeof *arrayOfStrings[i]?
    – chux
    Feb 8 at 15:03











  • @chux, I find it easier to get what is being sizeof'ed.
    – luizfzs
    Feb 8 at 15:17










Your Answer




StackExchange.ifUsing("editor", function ()
return StackExchange.using("mathjaxEditing", function ()
StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
);
);
, "mathjax-editing");

StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");

StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);

StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);

else
createEditor();

);

function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
convertImagesToLinks: false,
noModals: false,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);



);








 

draft saved


draft discarded


















StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186983%2ffinding-first-recurring-letter-in-each-string-of-a-given-set%23new-answer', 'question_page');

);

Post as a guest






























2 Answers
2






active

oldest

votes








2 Answers
2






active

oldest

votes









active

oldest

votes






active

oldest

votes








up vote
10
down vote



accepted










Better names



The variable names are too short to be meaningful. It's common to use i as an index in for-loops, but r doesn't really tell us its purpose. Names like number_of_strings are self-descriptive, so we should use those.



Unused variables



We never use c in find_first_recurring_letter, therefore we should remove the parameter altogether.



Strictly speaking, we also never use indx[i]. If ch[i] is '',indx[i]will be1(see "always use braces" below), and1` otherwise, so we can replace



 printf("'%c' in string %dn",ch[i],indx[i]+1);


by



 printf("'%c' in string %dn", ch[i], 2);


without changing the programs logic.



Simpler pointer logic



Instead of ptr = &str[i][0], which is ptr = &(*(str[i])) use ptr = str[i]. Instead of sptr = &ptr[1], just use sptr = ptr.



Again, we could call them current and next or similar.



By the way, all indx[i] are 1 (see "unused variables" above and "always use braces" below). That doesn't sound right. Also keep in mind that variable length arrays are optional from C11 on.



Get rid of additional memory



If we printf the recurring character, we can get rid of ch and indx:



 if(*ptr==*sptr)
printf("....");
indx[i]=1;


Always use braces



Note that the code above contains a bug. indx[1] will be set to 1 regardless whether *ptr == *sptr. The indentation is misleading here.



You probably added indx[i] later and forgot the add the braces, so we should always use braces to make sure that bugs like this cannot happen.



If you don't want to use braces, use a code formatter. The indentation on the automatically formatted code will hint possible errors.



Prefer late C99 declarations if possible



That way we can keep the scope of our variables short, e.g.



for(int i = 0; i < number_of_strings; ++i) 
for(char *current = str[i]; *current != ''; current++)
for(char *next = current + 1; *next != ''; next++)
if(*current == *next)
printf("'%c' in string %dn", *current, i);






It's now impossible to use next outside of its loop. Also, we should try to initialize our variables whenever possible.



Use const type* for inputs that are not supposed to change



That way we cannot accidentally modify your input values, e.g.



str[i][k] = ''


would not compile if str was const char**.



Check malloc's return



We should really check whether malloc returns NULL.



Split functions



find_first_recurring_letter doesn't follow its name completely. We find the first recurring letter on a set of strings on a per-string basis. That sounds perfect for a split:



const char * find_first_recurring_letter(const char * haystack) 
for(const char * current = haystack; *current != ''; current++)
for(const char * next = current + 1; *next != ''; next++)
if(*current == *next)
return current;



return NULL;


void find_first_recurring_letters(const char ** strings, int number_of_strings)
for(int i = 0; i < number_of_strings; i++)
const char * result = find_first_recurring_letter(strings[i]);

if(result)
printf("'%c' in string %dn", *result, i);





We can now use find_first_recurring_letter on any null-terminated string. We're able to reuse functionality. Note that both functions are very easy to check by hand.






share|improve this answer























  • but is there a better way of solving this problem @Zeta
    – katty
    Feb 7 at 11:59






  • 1




    @katty there are other algorithms with $mathcal O(n log k)$ time and $mathcal O(k)$ additional space (where k is the number of distinct characters), and a radix-like one with $mathcal O(n)$ time and $mathcal O(2 ^textsizeof(char))$ space. But those are other algorithms.
    – Zeta
    Feb 7 at 12:14















up vote
10
down vote



accepted










Better names



The variable names are too short to be meaningful. It's common to use i as an index in for-loops, but r doesn't really tell us its purpose. Names like number_of_strings are self-descriptive, so we should use those.



Unused variables



We never use c in find_first_recurring_letter, therefore we should remove the parameter altogether.



Strictly speaking, we also never use indx[i]. If ch[i] is '',indx[i]will be1(see "always use braces" below), and1` otherwise, so we can replace



 printf("'%c' in string %dn",ch[i],indx[i]+1);


by



 printf("'%c' in string %dn", ch[i], 2);


without changing the programs logic.



Simpler pointer logic



Instead of ptr = &str[i][0], which is ptr = &(*(str[i])) use ptr = str[i]. Instead of sptr = &ptr[1], just use sptr = ptr.



Again, we could call them current and next or similar.



By the way, all indx[i] are 1 (see "unused variables" above and "always use braces" below). That doesn't sound right. Also keep in mind that variable length arrays are optional from C11 on.



Get rid of additional memory



If we printf the recurring character, we can get rid of ch and indx:



 if(*ptr==*sptr)
printf("....");
indx[i]=1;


Always use braces



Note that the code above contains a bug. indx[1] will be set to 1 regardless whether *ptr == *sptr. The indentation is misleading here.



You probably added indx[i] later and forgot the add the braces, so we should always use braces to make sure that bugs like this cannot happen.



If you don't want to use braces, use a code formatter. The indentation on the automatically formatted code will hint possible errors.



Prefer late C99 declarations if possible



That way we can keep the scope of our variables short, e.g.



for(int i = 0; i < number_of_strings; ++i) 
for(char *current = str[i]; *current != ''; current++)
for(char *next = current + 1; *next != ''; next++)
if(*current == *next)
printf("'%c' in string %dn", *current, i);






It's now impossible to use next outside of its loop. Also, we should try to initialize our variables whenever possible.



Use const type* for inputs that are not supposed to change



That way we cannot accidentally modify your input values, e.g.



str[i][k] = ''


would not compile if str was const char**.



Check malloc's return



We should really check whether malloc returns NULL.



Split functions



find_first_recurring_letter doesn't follow its name completely. We find the first recurring letter on a set of strings on a per-string basis. That sounds perfect for a split:



const char * find_first_recurring_letter(const char * haystack) 
for(const char * current = haystack; *current != ''; current++)
for(const char * next = current + 1; *next != ''; next++)
if(*current == *next)
return current;



return NULL;


void find_first_recurring_letters(const char ** strings, int number_of_strings)
for(int i = 0; i < number_of_strings; i++)
const char * result = find_first_recurring_letter(strings[i]);

if(result)
printf("'%c' in string %dn", *result, i);





We can now use find_first_recurring_letter on any null-terminated string. We're able to reuse functionality. Note that both functions are very easy to check by hand.






share|improve this answer























  • but is there a better way of solving this problem @Zeta
    – katty
    Feb 7 at 11:59






  • 1




    @katty there are other algorithms with $mathcal O(n log k)$ time and $mathcal O(k)$ additional space (where k is the number of distinct characters), and a radix-like one with $mathcal O(n)$ time and $mathcal O(2 ^textsizeof(char))$ space. But those are other algorithms.
    – Zeta
    Feb 7 at 12:14













up vote
10
down vote



accepted







up vote
10
down vote



accepted






Better names



The variable names are too short to be meaningful. It's common to use i as an index in for-loops, but r doesn't really tell us its purpose. Names like number_of_strings are self-descriptive, so we should use those.



Unused variables



We never use c in find_first_recurring_letter, therefore we should remove the parameter altogether.



Strictly speaking, we also never use indx[i]. If ch[i] is '',indx[i]will be1(see "always use braces" below), and1` otherwise, so we can replace



 printf("'%c' in string %dn",ch[i],indx[i]+1);


by



 printf("'%c' in string %dn", ch[i], 2);


without changing the programs logic.



Simpler pointer logic



Instead of ptr = &str[i][0], which is ptr = &(*(str[i])) use ptr = str[i]. Instead of sptr = &ptr[1], just use sptr = ptr.



Again, we could call them current and next or similar.



By the way, all indx[i] are 1 (see "unused variables" above and "always use braces" below). That doesn't sound right. Also keep in mind that variable length arrays are optional from C11 on.



Get rid of additional memory



If we printf the recurring character, we can get rid of ch and indx:



 if(*ptr==*sptr)
printf("....");
indx[i]=1;


Always use braces



Note that the code above contains a bug. indx[1] will be set to 1 regardless whether *ptr == *sptr. The indentation is misleading here.



You probably added indx[i] later and forgot the add the braces, so we should always use braces to make sure that bugs like this cannot happen.



If you don't want to use braces, use a code formatter. The indentation on the automatically formatted code will hint possible errors.



Prefer late C99 declarations if possible



That way we can keep the scope of our variables short, e.g.



for(int i = 0; i < number_of_strings; ++i) 
for(char *current = str[i]; *current != ''; current++)
for(char *next = current + 1; *next != ''; next++)
if(*current == *next)
printf("'%c' in string %dn", *current, i);






It's now impossible to use next outside of its loop. Also, we should try to initialize our variables whenever possible.



Use const type* for inputs that are not supposed to change



That way we cannot accidentally modify your input values, e.g.



str[i][k] = ''


would not compile if str was const char**.



Check malloc's return



We should really check whether malloc returns NULL.



Split functions



find_first_recurring_letter doesn't follow its name completely. We find the first recurring letter on a set of strings on a per-string basis. That sounds perfect for a split:



const char * find_first_recurring_letter(const char * haystack) 
for(const char * current = haystack; *current != ''; current++)
for(const char * next = current + 1; *next != ''; next++)
if(*current == *next)
return current;



return NULL;


void find_first_recurring_letters(const char ** strings, int number_of_strings)
for(int i = 0; i < number_of_strings; i++)
const char * result = find_first_recurring_letter(strings[i]);

if(result)
printf("'%c' in string %dn", *result, i);





We can now use find_first_recurring_letter on any null-terminated string. We're able to reuse functionality. Note that both functions are very easy to check by hand.






share|improve this answer















Better names



The variable names are too short to be meaningful. It's common to use i as an index in for-loops, but r doesn't really tell us its purpose. Names like number_of_strings are self-descriptive, so we should use those.



Unused variables



We never use c in find_first_recurring_letter, therefore we should remove the parameter altogether.



Strictly speaking, we also never use indx[i]. If ch[i] is '',indx[i]will be1(see "always use braces" below), and1` otherwise, so we can replace



 printf("'%c' in string %dn",ch[i],indx[i]+1);


by



 printf("'%c' in string %dn", ch[i], 2);


without changing the programs logic.



Simpler pointer logic



Instead of ptr = &str[i][0], which is ptr = &(*(str[i])) use ptr = str[i]. Instead of sptr = &ptr[1], just use sptr = ptr.



Again, we could call them current and next or similar.



By the way, all indx[i] are 1 (see "unused variables" above and "always use braces" below). That doesn't sound right. Also keep in mind that variable length arrays are optional from C11 on.



Get rid of additional memory



If we printf the recurring character, we can get rid of ch and indx:



 if(*ptr==*sptr)
printf("....");
indx[i]=1;


Always use braces



Note that the code above contains a bug. indx[1] will be set to 1 regardless whether *ptr == *sptr. The indentation is misleading here.



You probably added indx[i] later and forgot the add the braces, so we should always use braces to make sure that bugs like this cannot happen.



If you don't want to use braces, use a code formatter. The indentation on the automatically formatted code will hint possible errors.



Prefer late C99 declarations if possible



That way we can keep the scope of our variables short, e.g.



for(int i = 0; i < number_of_strings; ++i) 
for(char *current = str[i]; *current != ''; current++)
for(char *next = current + 1; *next != ''; next++)
if(*current == *next)
printf("'%c' in string %dn", *current, i);






It's now impossible to use next outside of its loop. Also, we should try to initialize our variables whenever possible.



Use const type* for inputs that are not supposed to change



That way we cannot accidentally modify your input values, e.g.



str[i][k] = ''


would not compile if str was const char**.



Check malloc's return



We should really check whether malloc returns NULL.



Split functions



find_first_recurring_letter doesn't follow its name completely. We find the first recurring letter on a set of strings on a per-string basis. That sounds perfect for a split:



const char * find_first_recurring_letter(const char * haystack) 
for(const char * current = haystack; *current != ''; current++)
for(const char * next = current + 1; *next != ''; next++)
if(*current == *next)
return current;



return NULL;


void find_first_recurring_letters(const char ** strings, int number_of_strings)
for(int i = 0; i < number_of_strings; i++)
const char * result = find_first_recurring_letter(strings[i]);

if(result)
printf("'%c' in string %dn", *result, i);





We can now use find_first_recurring_letter on any null-terminated string. We're able to reuse functionality. Note that both functions are very easy to check by hand.







share|improve this answer















share|improve this answer



share|improve this answer








edited Feb 7 at 16:18


























answered Feb 7 at 9:35









Zeta

14.3k23267




14.3k23267











  • but is there a better way of solving this problem @Zeta
    – katty
    Feb 7 at 11:59






  • 1




    @katty there are other algorithms with $mathcal O(n log k)$ time and $mathcal O(k)$ additional space (where k is the number of distinct characters), and a radix-like one with $mathcal O(n)$ time and $mathcal O(2 ^textsizeof(char))$ space. But those are other algorithms.
    – Zeta
    Feb 7 at 12:14

















  • but is there a better way of solving this problem @Zeta
    – katty
    Feb 7 at 11:59






  • 1




    @katty there are other algorithms with $mathcal O(n log k)$ time and $mathcal O(k)$ additional space (where k is the number of distinct characters), and a radix-like one with $mathcal O(n)$ time and $mathcal O(2 ^textsizeof(char))$ space. But those are other algorithms.
    – Zeta
    Feb 7 at 12:14
















but is there a better way of solving this problem @Zeta
– katty
Feb 7 at 11:59




but is there a better way of solving this problem @Zeta
– katty
Feb 7 at 11:59




1




1




@katty there are other algorithms with $mathcal O(n log k)$ time and $mathcal O(k)$ additional space (where k is the number of distinct characters), and a radix-like one with $mathcal O(n)$ time and $mathcal O(2 ^textsizeof(char))$ space. But those are other algorithms.
– Zeta
Feb 7 at 12:14





@katty there are other algorithms with $mathcal O(n log k)$ time and $mathcal O(k)$ additional space (where k is the number of distinct characters), and a radix-like one with $mathcal O(n)$ time and $mathcal O(2 ^textsizeof(char))$ space. But those are other algorithms.
– Zeta
Feb 7 at 12:14













up vote
3
down vote













@Zeta's answer is very good but I'd go ahead and improve the code by creating a few functions to improve readability:



void allocMemoryForStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
arrayOfStrings[i] = malloc(sizeof(*arrayOfStrings[i]) * 50);



void printStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
printf("%sn", arrayOfStrings[i]);



void freeStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
free(arrayOfStrings[i]);




P.S.: The code might need some tweaks to compile/work. Use it as a reference.






share|improve this answer



















  • 1




    "some tweaks"? You didn't rename str or r in your second and third example, and i wasn't declared at all.
    – Zeta
    Feb 7 at 16:19










  • @Zeta you're absolutely right. I've corrected that and also changed the function names to cameCase.
    – luizfzs
    Feb 7 at 18:07






  • 1




    arrayOfStrings[i] = (char*) malloc(50 * sizeof(char)); ---> cast not needed. Also, consider using the size of the reference type and not an explicit type.: arrayOfStrings[i] = malloc(sizeof *arrayOfStrings[i] * 50);. Easy to code. review and maintain.
    – chux
    Feb 7 at 19:25










  • @luizfzs Curious, do you find sizeof(*arrayOfStrings[i]) easier to read than without parens: sizeof *arrayOfStrings[i]?
    – chux
    Feb 8 at 15:03











  • @chux, I find it easier to get what is being sizeof'ed.
    – luizfzs
    Feb 8 at 15:17














up vote
3
down vote













@Zeta's answer is very good but I'd go ahead and improve the code by creating a few functions to improve readability:



void allocMemoryForStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
arrayOfStrings[i] = malloc(sizeof(*arrayOfStrings[i]) * 50);



void printStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
printf("%sn", arrayOfStrings[i]);



void freeStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
free(arrayOfStrings[i]);




P.S.: The code might need some tweaks to compile/work. Use it as a reference.






share|improve this answer



















  • 1




    "some tweaks"? You didn't rename str or r in your second and third example, and i wasn't declared at all.
    – Zeta
    Feb 7 at 16:19










  • @Zeta you're absolutely right. I've corrected that and also changed the function names to cameCase.
    – luizfzs
    Feb 7 at 18:07






  • 1




    arrayOfStrings[i] = (char*) malloc(50 * sizeof(char)); ---> cast not needed. Also, consider using the size of the reference type and not an explicit type.: arrayOfStrings[i] = malloc(sizeof *arrayOfStrings[i] * 50);. Easy to code. review and maintain.
    – chux
    Feb 7 at 19:25










  • @luizfzs Curious, do you find sizeof(*arrayOfStrings[i]) easier to read than without parens: sizeof *arrayOfStrings[i]?
    – chux
    Feb 8 at 15:03











  • @chux, I find it easier to get what is being sizeof'ed.
    – luizfzs
    Feb 8 at 15:17












up vote
3
down vote










up vote
3
down vote









@Zeta's answer is very good but I'd go ahead and improve the code by creating a few functions to improve readability:



void allocMemoryForStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
arrayOfStrings[i] = malloc(sizeof(*arrayOfStrings[i]) * 50);



void printStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
printf("%sn", arrayOfStrings[i]);



void freeStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
free(arrayOfStrings[i]);




P.S.: The code might need some tweaks to compile/work. Use it as a reference.






share|improve this answer















@Zeta's answer is very good but I'd go ahead and improve the code by creating a few functions to improve readability:



void allocMemoryForStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
arrayOfStrings[i] = malloc(sizeof(*arrayOfStrings[i]) * 50);



void printStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
printf("%sn", arrayOfStrings[i]);



void freeStrings(char** arrayOfStrings, int numOfStrings)
for(int i = 0; i < numOfStrings; i++)
free(arrayOfStrings[i]);




P.S.: The code might need some tweaks to compile/work. Use it as a reference.







share|improve this answer















share|improve this answer



share|improve this answer








edited Feb 8 at 11:30


























answered Feb 7 at 13:58









luizfzs

1314




1314







  • 1




    "some tweaks"? You didn't rename str or r in your second and third example, and i wasn't declared at all.
    – Zeta
    Feb 7 at 16:19










  • @Zeta you're absolutely right. I've corrected that and also changed the function names to cameCase.
    – luizfzs
    Feb 7 at 18:07






  • 1




    arrayOfStrings[i] = (char*) malloc(50 * sizeof(char)); ---> cast not needed. Also, consider using the size of the reference type and not an explicit type.: arrayOfStrings[i] = malloc(sizeof *arrayOfStrings[i] * 50);. Easy to code. review and maintain.
    – chux
    Feb 7 at 19:25










  • @luizfzs Curious, do you find sizeof(*arrayOfStrings[i]) easier to read than without parens: sizeof *arrayOfStrings[i]?
    – chux
    Feb 8 at 15:03











  • @chux, I find it easier to get what is being sizeof'ed.
    – luizfzs
    Feb 8 at 15:17












  • 1




    "some tweaks"? You didn't rename str or r in your second and third example, and i wasn't declared at all.
    – Zeta
    Feb 7 at 16:19










  • @Zeta you're absolutely right. I've corrected that and also changed the function names to cameCase.
    – luizfzs
    Feb 7 at 18:07






  • 1




    arrayOfStrings[i] = (char*) malloc(50 * sizeof(char)); ---> cast not needed. Also, consider using the size of the reference type and not an explicit type.: arrayOfStrings[i] = malloc(sizeof *arrayOfStrings[i] * 50);. Easy to code. review and maintain.
    – chux
    Feb 7 at 19:25










  • @luizfzs Curious, do you find sizeof(*arrayOfStrings[i]) easier to read than without parens: sizeof *arrayOfStrings[i]?
    – chux
    Feb 8 at 15:03











  • @chux, I find it easier to get what is being sizeof'ed.
    – luizfzs
    Feb 8 at 15:17







1




1




"some tweaks"? You didn't rename str or r in your second and third example, and i wasn't declared at all.
– Zeta
Feb 7 at 16:19




"some tweaks"? You didn't rename str or r in your second and third example, and i wasn't declared at all.
– Zeta
Feb 7 at 16:19












@Zeta you're absolutely right. I've corrected that and also changed the function names to cameCase.
– luizfzs
Feb 7 at 18:07




@Zeta you're absolutely right. I've corrected that and also changed the function names to cameCase.
– luizfzs
Feb 7 at 18:07




1




1




arrayOfStrings[i] = (char*) malloc(50 * sizeof(char)); ---> cast not needed. Also, consider using the size of the reference type and not an explicit type.: arrayOfStrings[i] = malloc(sizeof *arrayOfStrings[i] * 50);. Easy to code. review and maintain.
– chux
Feb 7 at 19:25




arrayOfStrings[i] = (char*) malloc(50 * sizeof(char)); ---> cast not needed. Also, consider using the size of the reference type and not an explicit type.: arrayOfStrings[i] = malloc(sizeof *arrayOfStrings[i] * 50);. Easy to code. review and maintain.
– chux
Feb 7 at 19:25












@luizfzs Curious, do you find sizeof(*arrayOfStrings[i]) easier to read than without parens: sizeof *arrayOfStrings[i]?
– chux
Feb 8 at 15:03





@luizfzs Curious, do you find sizeof(*arrayOfStrings[i]) easier to read than without parens: sizeof *arrayOfStrings[i]?
– chux
Feb 8 at 15:03













@chux, I find it easier to get what is being sizeof'ed.
– luizfzs
Feb 8 at 15:17




@chux, I find it easier to get what is being sizeof'ed.
– luizfzs
Feb 8 at 15:17












 

draft saved


draft discarded


























 


draft saved


draft discarded














StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f186983%2ffinding-first-recurring-letter-in-each-string-of-a-given-set%23new-answer', 'question_page');

);

Post as a guest













































































Popular posts from this blog

Chat program with C++ and SFML

Function to Return a JSON Like Objects Using VBA Collections and Arrays

Will my employers contract hold up in court?