7
\$\begingroup\$

This piece of code works fine. But I'm wondering if it can be done in a more efficient way. More specifically, this part (*(s1 + i)) if it possible to force it to sequence through entire array character by character via pointer, for example, *s1++.

My task to do this function compareStrings without index array []:

int  compareStrings(const char  *s1, const char  *s2)
{
    int i = 0, answer;
    //  i - to sequence through array of characters
    // pointer to character string1 and character string2
    while (*(s1 + i) == *(s2 + i) && *(s1 + i) != '\0'&& *(s2 + i) != '\0')
    {
        i++;
    }

    if ( *(s1 + i) < *(s2 + i) )
        answer = -1;               /* s1 < s2  */
    else if ( *(s1 + i) == *(s2 + i) )
            answer = 0;                 /* s1 == s2 */
        else
            answer = 1;                 /* s1 > s2  */

        return answer;

But I want to change it to s1++ and s2++ instead of *(s1 + i) and *(s2 + i). I've tried to implement this idea with pining an extra pointer to the beginning but I've failed.

int  compareStrings(const char  *s1, const char  *s2)
{
  int answer;
  char **i = s1, **j = s2;
  // i to sequence through array of characters
  while (*(i++) == *(j++) && *(i++) != '\0'&& *(j++) != '\0');

  if (*i < *j)
      answer = -1;               /* s1 < s2  */
  else if (*i == *j)
      answer = 0;                 /* s1 == s2 */
  else
      answer = 1;                 /* s1 > s2  */

  return answer;
}
\$\endgroup\$
  • \$\begingroup\$ Your braces and indentation are off. Please check that your code is posted as intended. \$\endgroup\$ – 200_success Feb 3 '17 at 23:14
  • 1
    \$\begingroup\$ I'm pretty sure *(s1 + i) == *s2 + i is a typo and you mean *(s1 + i) == *(s2 + i) \$\endgroup\$ – Buge Feb 4 '17 at 8:42
16
\$\begingroup\$

You don't need pointers to character pointers at all:

int str_cmp(const char* s1, const char* s2)
{
    while (*s1 != '\0' && *s1 == *s2)
    {
        ++s1;
        ++s2;
    }

    if (*s1 == *s2)
    {
        return 0;
    }

    return *s1 < *s2 ? -1 : 1;
}

Also, there is a bug in your second implementation: it returns 1 on compareStrings("hello", "helloo"), when the correct result is -1.

Hope that helps.

\$\endgroup\$
  • 2
    \$\begingroup\$ It would be more understandable if you write the test for end-of-string as "*s != '\0'". \$\endgroup\$ – jamesqf Feb 4 '17 at 6:10
  • 2
    \$\begingroup\$ You don't need to check that both *s1 and *s2 are non-null. Since you are checking that they are equal, you only need to check one of them for non-null. Although as written the compiler would probably optimize it to only do 1 check. \$\endgroup\$ – Buge Feb 4 '17 at 8:44
4
\$\begingroup\$

There's another huge bug in your code which stems from a quirk of C (and C++):
Plain char can be either signed or unsigned.

Next, do you really want to normalize the return-values to one of -1, 0 and 1, or is negative, zero, positive enough? The latter is what the standard does... and it's less work.

int compareStrings(const char* s1, const char* s2)
{
    while (*s1 && *s1 == *s2) {
        ++s1;
        ++s2;
    }
    return (int)(unsigned char)*s1 - (int)(unsigned char)*s2; // not normalized
    return *s1 == *s2 ? 0 : (unsigned char)*s1 < (unsigned char)*s2 ? -1 : 1; // normalized
}

(The code assumes that an int is bigger than a char.)

\$\endgroup\$
  • \$\begingroup\$ I've tried to implement your version of this function and it doesn't do the job that I need to at all. @Deduplicator \$\endgroup\$ – Yellowfun Feb 3 '17 at 16:38
  • 2
    \$\begingroup\$ This wrong. If string s2 is shorter than s1 you read past the end of s2. \$\endgroup\$ – Tonny Feb 3 '17 at 16:38
  • 3
    \$\begingroup\$ @Tony: How? That would mean they are different and thus stops the loop! \$\endgroup\$ – Deduplicator Feb 3 '17 at 17:58
  • \$\begingroup\$ @Yellowfun: You mean you have to have your return-value normalized, or what? \$\endgroup\$ – Deduplicator Feb 3 '17 at 18:00
  • \$\begingroup\$ Now it's working fine! @Deduplicator \$\endgroup\$ – Yellowfun Feb 3 '17 at 18:08
3
\$\begingroup\$

Remember when you learned that arrays are nothing but the pointer to the first element of the same array? You can do this the other way around too: *(s1 + 1) is equivalent to s1[1].

In the second example take a look at how you resolve which pointer. **i = s1 so will *(i++) be equal to *(s1 + 1)? Your debugger can tell you the details from here :)

\$\endgroup\$
  • \$\begingroup\$ It says "+ i 0x0018e25c {0x69726561 <Error reading characters of string.>} char * * " \$\endgroup\$ – Yellowfun Feb 3 '17 at 12:26
  • \$\begingroup\$ About it I know '*(s1 + 1) is equivalent to s1[1]'. :) \$\endgroup\$ – Yellowfun Feb 3 '17 at 12:30
  • 5
    \$\begingroup\$ "Remember when you learned that arrays are nothing but the pointer to the first element of the same array?" If so, then you were taught incorrectly. :-) What you say here is true, but it's because arrays decay implicitly to a pointer to their first element. The two things are not equivalent as far as the language is concerned. \$\endgroup\$ – Cody Gray Feb 3 '17 at 13:07
  • 1
    \$\begingroup\$ Agreed, it is an oversimplification. \$\endgroup\$ – fer-rum Feb 3 '17 at 13:33
  • \$\begingroup\$ Thank you. I thought pointer to array is a & address in the memory to that array[]. But we can reach to it from start to end through the first element and than incrementing with something like index. \$\endgroup\$ – Yellowfun Feb 3 '17 at 16:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.