Which string is the longest-CodePudding

My code: What I'm trying to do is to input two strings, then return the longest one. If they're the same length then return NULL. Now, the code is just outputting gibberish and I cannot find out why. The function returns a pointer to the first character of the largest string. Then it goes through the while loop, and I'm trying to dereference the pointer and print out its value.

Note: I'm revising for an exam and we have to use only pointers and not treat strings as arrays.

#include<stdio.h>

char* string_ln(char*, char*);
 
int main() {
    char str1[20];
    char str2[20];
    char* length;
   
    scanf("%s%s", str1, str2);
   
    length = string_ln(str1, str2);
   
    while (length != '\0') {
        printf("%c", *length);
        length  ;
    }
}
 
char* string_ln(char*p1, char*p2) {
    int count1 = 0;
    while (*p1 != '\0') {
        count1  ;
        p1  ;
    }
   
    int count2 = 0;
    while (*p2 != '\0') {
        count2  ;
        p2  ;
    }
   
    if (count1 > count2) {
        return p1;
    }
    else if (count2 > count1) {
        return p2;
    }
    else {
        return NULL;
    }
}

CodePudding user response：

In writing string_ln you iterate over both strings completely to find their lengths, and then compare those numbers. This can work, but you don't actually need to do this. You only need to know which is longer. It doesn't matter how much longer the longer string is.

char *string_ln(char *str1, char *str2) {
    char *iter1, *iter2;

    for (iter1 = str1, iter2 = str2;
         *iter1 && *iter2;
         iter1  , iter2  );

    if (!(*iter1 || *iter2)) {
        return NULL;
    }
    else if (*iter1) {
        return str1;
    }
    else {
        return str2;
    }
}

We simply need to iterate over both strings, until at least one hits a NULL character. Once we get to that point, we can test to see which iterator is NULL. If it's both of them, then they're the same length. If the first iterator is not NULL, then the first string is longer. Otherwise, the second string is longer.

The benefit to this approach is that we avoid unnecessary work, and make it much quicker to compare strings of very different lengths.

CodePudding user response：

I think you're missing to dereference the pointer. Instead of

while(length!='\0')

you'd need

while(*length!='\0')

That said, in the called function, you're reuring pointers after the increment, i.e., the returned pointers do not point to the start of the string anymore. You need to ensure that you return pointers which points to the beginning of the string. You can change your code to

int count1 = 0;
while (p1[count1] != '\0') {
   count1  ;
}

int count2 = 0;
while (p2[count2] != '\0') {
   count2  ;
}

so that p1 and p2 does not change.

CodePudding user response：

There are a few problems here. First, you're modifying p1 and p2 in the function, so you won't actually return a pointer to the beginning of the largest string, but to its end. One way to avoid this is to iterate over copies of p1 and p2:

char* string_ln(char*p1, char*p2)
{
   char* tmp1 = p1;
   int count1 = 0;
   while (*tmp1 != '\0') {
      count1  ;
      tmp1  ;
   }
   
   char* tmp2 = p2;
   int count2 = 0;
   while (*tmp2 != '\0') {
      count2  ;
      tmp2  ;
   }
   
   if(count1>count2){
       return p1;
   }
   else if(count2>count1){
       return p2;
   }
   else{
       return NULL;
   }
}

Second, in your main, you're using the %c format string, which works for a single char, not a whole string. Since you have a string anyway, you can avoid a format string and just print it directly. Also, note that you should explicitly check for NULLs:

int main() {
   char str1[20];
   char str2[20];
   char* longest;
   
   scanf("%s%s", str1, str2);
   
   longest = string_ln(str1, str2);
   
   if (longest) {
       printf(longest);
   } else {
       printf("They are the same length");
   }
}

CodePudding user response：

For starters the function should be declared like

char * string_ln( const char *, const char * );

because the passed strings are not being changed within the function.

You are returning from the function the already modified pointer p1 or p2 that is being changed in one of the while loops

while (*p1 != '\0') {
   count1  ;
   p1  ;
}

while (*p2 != '\0') {
   count2  ;
   p2  ;
}

So the returned pointer points to the terminating zero '\0' of a string.

Moreover in main before this while loop

length = string_ln(str1, str2);

while(length!='\0'){ printf("%c", *length); length ; }

you are not checking whether the pointer length is equal to NULL. As a result the program can invoke undefined behavior.

The function itself can be defined the following way using only pointers.

char * string_ln( const char *p1, const char *p2 )
{
    const char *s1 = p1;
    const char *s2 = p2;

    while ( *s1 != '\0' && *s2 != '\0' )
    {
          s1;
          s2;
    }

    if ( *s1 == *s2 )
    {
        return NULL;
    }
    else if ( *s1 == '\0' )
    {
        return ( char * )p2;
    }
    else
    {
        return ( char * )p1;
    }        
}

and in main you need to write

char *length = string_ln( str1, str2 );

if ( length != NULL )
{
    while ( *length )
    printf( "%c", *length   );
}

Pay attention to that the return type of the function is char * instead of const char *. It is because in C there is no function overloading and the returned pointer can point to a constant string or to a non-constant string. It is a general convention in C for declaring string functions.