CODEWITHSUNDEEP
cstring
Ma250
Ma250

Reputation: 357

Why code shows that strings are the same when one string is greater than the other

I wrote a function that replaces the function strcmp().
The cases are:

1) strings are the same
2) second string will come first in the dictionary.
3) first string will come first in the dictionary.

In theory:

'a' > 'b'

So 'a' is the first string to come in the dictionary, however, my code doesn't exactly view it like this, instead it treats it like it's case 1.

Here is my code: 

int cmp(char fString[], char sString[])
{
    int flag = 0;
    int i = 0;

    for (i = 0; fString[i]; i++) {
        if (fString[i] == sString[i]) {
            flag = 0;
        } else
        if (fString[i] > sString[i]) {
            flag = 1;
        } else {
            flag = -1;
        }
    }
    return flag;
}

The conditions are:

if (cmp(fString, sString) == 0) {
    printf("Strings are the same.\n");
} else
if (cmp(fString, sString) > 0) {
    printf("First string will come first in the dictionary\n");
} else {
    printf("Second string will come first in the dictionary\n");
}

Where did I do wrong?

Upvotes: 0

Views: 104

Answers (3)

chqrlie
chqrlie

Reputation: 144780

Your assumption is false: 'a' > 'b' should instead read

'a' < 'b'

If you insist on writing a function that returns 1 if the first string argument should come before the second argument, you should at least implement the correct algorithm:

  • you must stop the iteration when you encounter characters that differ. You currently keep looping and overwrite flag with the value for the next character.
  • you must deal with the case where the first string is shorter than the second, you currently return 0, which is incorrect.

Here is a corrected version:

int cmp(char fString[], char sString[]) {
    int i = 0;

    for (i = 0; fString[i]; i++) {
        if (fString[i] == sString[i]) {
            continue;
        } else
        if (fString[i] > sString[i]) {
            return 1;
        } else {
            return -1;
        }
    }
    if (sString[i])
        return 1;  // fString is shorter, it should come first 
    else
        return 0;  // strings are the same
}

Note however that it is very confusing to use counter-intuitive conventions. A string comparison function should return < 0 is its first argument is conceptually less the the second and should come first in a list sorted by increasing order.

Note also that the strcmp() function compares the strings according to the order of the type unsigned char instead of char.

Upvotes: 2

Mohsen_Fatemi
Mohsen_Fatemi

Reputation: 3401

first you must try to handle this problem , the length of both Strings are equal ? if not you , must work with minimum

notice that your implemented cmp function only works when the strings are equal

here is the problem of your cmp function , you must break your for loop when you know one String is bigger or less than other string , but your code continues checking remaining characters :

for(i = 0; fString[i]; i++)
    if(fString[i] == sString[i])
        flag = 0;
    else if(fString[i] > sString[i]){
        flag = 1;
        break;
    }else{
        flag = -1;
        break;
    }

if i was you , first i assume the strings are equal , then i would try to change the flag if String is greater than or less than other string

int flag = 0 , i = 0;
for(i = 0; fString[i]; i++)
        if(fString[i] > sString[i]){
            flag = 1;
            break;
        }else{
            flag = -1;
            break;
        }

Upvotes: 1

Iharob Al Asimi
Iharob Al Asimi

Reputation: 53006

Your algorithm is wrong, you should return the difference between the characters when it's not 0 instead. Because the flag is overwritten if the last characters are the same, something very easy to happen if you include a trailing '\n' in your strings.

Note that by last I mean, the last character of the shortest string is the same as that of the longer string at the given position.

One quick fix is to check for flag == 0 in the for loop condition, but you can simply check if the difference between the two characters is 0 and continue with the next character or return it if it's not 0.

Upvotes: 0

Related Questions