2
\$\begingroup\$

I'm learning C and have made this very simple function to compare two strings. I would like to know how it can be improved:

int match(char* string1, char* string2)
{
    size_t i = 0;

    while (string1[i] != '\0' || string2[i] != '\0') {
        if (string1[i] != string2[i]) {
            return 0;
        }
        ++i;
    }

    return 1;
}

I figured it will only do 2 comparisons per iteration, and 3 in the last iteration. Also, I believe it won't go beyond the end of any array because it will return before iterating beyond the null terminator.

\$\endgroup\$
4
\$\begingroup\$

Your parameters should be const and your loop need only to check one string for the terminator. For example in the following loop, if *s1 is 0 the loop stops because of the first condition. If *s1 is not 0 but *s2 is 0, the loop stops because of the 2nd condition.

int match(const char *s1, const char *s2)
{
    while (*s1 && (*s1++ == *s2++)) {
         // nothing
    }
    return *s1 - *s2;
}
| improve this answer | |
\$\endgroup\$
1
\$\begingroup\$

In your while loop, you check whether string1[i] or string2[i] are not null. If they are of unequal lengths, they will have their null terminators at different values of i; because at least one is non-null at each point, your while loop would then continue forever. To fix this, all you have to do is change || to &&, because you need to make sure that both of your characters are valid.

Another improvement that I would consider would be changing the return value to be zero if they are equal; this would match the built-in strcmp function. Then, you could return i as the place at which the two strings diverge, also matching strcmp. This provides the programmer with more useful information from one function call without a loss in value, because 0 is false and everything else is true.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Don't worry, the termination works fine. If they are unequal length, the if-comparison inside the while-loop will be triggered, and return 0. \$\endgroup\$ – 200_success Sep 13 '13 at 5:33
  • \$\begingroup\$ Right, I didn't consider that. In any case, it would be more readable for a && operator. Also, if OP cares about performance, he should know that && is short-circuiting: It will only evaluate the right operand if the left operand is true, providing some (small) performance gain. \$\endgroup\$ – acrucker Sep 13 '13 at 19:53
  • \$\begingroup\$ Once you change || to &&, it no longer works correctly. When one string begins with the other, it would report them as being equal. \$\endgroup\$ – 200_success Sep 13 '13 at 21:05

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.