Compare two character strings in C

Question

Is there any problem in my code? I found so many codes on internet but they all complex and I wrote my code and its work on my tests. Do you see any problem in this code?

bool is_equal(char str1[], char str2[])
{
    int i;
    bool isEqual;

    for(i=0; str1[i] != '\0' || str2[i] !='\0'; i++)
    {
        if(str1[i] != str2[i])
            isEqual = false;
        else
            isEqual = true;
    }

    return isEqual;
}

yes, once you set `isEqual` to `false` you don't want to change it to `true` later. Better return `false` immediately ... and `return true;` at the end of the loop if you reach it — pmg, Jun 10 '20 at 10:49
1) What will your code return if passed the strings `"abcd"` and `"aXcd"`? 2) What happens if it is passed `"abcd"` and `"abc"`? — gspr, Jun 10 '20 at 10:51
There are *many* errors in this code. I also wonder what is so “complex” about the code you’ve found online, since this is, fundamentally, an easy problem with a simple, established solution (and let’s not forget that `strcmp(a, b) == 0` exists). — Konrad Rudolph, Jun 10 '20 at 10:53
Also: initialize `isEqual` to `false`, and change your loop condition from `||` to `&&`. — jarmod, Jun 10 '20 at 10:54
Rather than asking other people, a better way of testing would be to use a unit test library, and write some tests. And then run them with whatever sanitizers (memory, race, ...) you can find. Comparing all pairs of `""`, `"a"`, `"aa"`, `"aaa"`, `"aab"`, `"baa"` should find all trivial bugs. — Paul Hankin, Jun 10 '20 at 11:13
@PaulHankin Well, what you are saying is not false. But tbh, do you think it is easier to write a test that compares all pairs of a set of strings than it is to write a string comparison function? ;) — klutt, Jun 10 '20 at 11:17
You say that it works on your tests. Then you should also provide some tests. — klutt, Jun 10 '20 at 11:22
Go back to pen and paper. Before you know how the algorithm should work, it's too early to write code. — klutt, Jun 10 '20 at 11:35
You posted a "solution" in an answer you deleted. That was still buggy. It would return equal for all strings of same length as long as the last character was the same. — klutt, Jun 10 '20 at 11:36

klutt · Accepted Answer · 2020-06-10T14:50:49.837

Ok, just to put an end to this. Let's think what string comparing is. First, let's disregard NULL pointers, and assume that both strings are actually valid strings. Then we need no special check.

We will do this by simply comparing them element for element. Whenever we compare two elements at the same index, one of three things can happen:

Both elements are '\0'. The strings are equal. Return true.
The elements differ. Stop comparing and return false.
The elements are equal, but not '\0'. Continue comparing.

Note that the case where "one string has ended" is included in case 2. If only one string has ended, then we will compare '\0' with something else, and thus they will differ.

To make the code very easy to write, you can check if the length is the same first. It makes it a little bit easier, since if they have different length, then they are different, and if they have the same length, you can loop from zero to that length. You could then write it very simple like this:

bool is_equal(char str1[], char str2[])
{
    if(strlen(str1) != strlen(str2))
        return false;

    for(int i=0; i<strlen(str1); i++) {
        if(str1[i] != str2[i])
            return false;
    }

    return true;
}

Most important thing here that differs from your code is the realization that as soon as you discover ANY difference, you can return false. The above code is very clear, but a bit inefficient. An obvious optimization is this:

bool is_equal(char str1[], char str2[])
{
    int len1 = strlen(str1);
    if(len1 != strlen(str2))
        return false;

    for(int i=0; i<len1; i++) {
        if(str1[i] != str2[i])
            return false;
    }

    return true;
}

This removes unnecessary length check in the loop, but it looks messier. Actually, I think I would prefer this:

    int len1 = strlen(str1);
    int len2 = strlen(str1);
    if(len1 != len2)
        return false;

because it looks cleaner. But it's an extra line and an extra variable. But since we can combine the length check with the character check, we can get rid of that completely.

So how can we incorporate the length check in the character check? Well, like this. I want to point out that !str[i] and str[i] != '\0' is completely equivalent in this situation, and it's a very common way to check end of strings.

bool is_equal(char str1[], char str2[])
{
    // Infinite loop, but it does not matter. It will end when it should.
    for(int i=0; ; i++) {
        // If the strings are different, they are different
        if(str1[i] != str2[i])
            return false;
        // At this stage we know that they are equal for all elements
        // checked so far, so if both strings ends here, then they are equal.
        if(!str1[i] && !str2[i])
            return true;
    }
}

This is not how I would have implemented it, and I omitted a few details. For instance, I would have used size_t instead of int for the index variable and const for the pointers. It's intended to show how it works in a very simple way. If I implemented it, I would do something like this:

bool is_equal(const char *str1, const char *str2)
{
    while(true) {
        if(*str1 != *str2)
            return false;
        if(!*str1 && !*str2)
            return true;
        str1++;
        str2++;
    }
}

If you want to work with index variables or pointers is often just a matter of taste. Personally, I think pointers is a bit clearer in this situation. I would also consider adding these lines in the beginning, before the loop:

assert(str1);
assert(str2);

Those will make the program crash if you feed them a NULL pointer.

Demonstration:

https://onlinegdb.com/r1XvzLR3I

@Magic Both would work. But if the strings have equal length, it's not necessary to compare the terminators. — klutt, Jun 11 '20 at 09:16

Compare two character strings in C

1 Answers1