0

My program compares the 2 strings entirely and does not stop once n number of characters are reached? Why does this happen?

int strncompare (const char* mystring1,const char* mystring2, int number)
{
    int z;
    z = number - 1;
    while ((*mystring1==*mystring2) && (*mystring1 != '\0') && (*mystring2 != '\0'))
    {
        *mystring1++;
        *mystring2++;
        if ((*mystring1 == mystring1[z]) && (*mystring2 == mystring2[z])) 
        {
            break;
        }
    }
    return (mystring1++ - mystring2++);
    }
Apsara Sriram
  • 1
  • 4
    Because you don't stop when you compare `number` characters? Also why are you doing `*mystring1++;`? You don't need the `*`. Please read a good C++ book before continuing. – Seth Carnegie Feb 14 '12 at 22:36
  • 1
    Not directly related to your question, but what do you think `*mystring1++;` and `*mystring2++;` do? I'm guessing it isn't what they actually do. –  Feb 14 '12 at 22:36
  • 1
    @SethCarnegie The `if ((*mystring1 == mystring1[z]) && (*mystring2 == mystring2[z]))` is supposed to check for that, but the check is wrong in its logic and its implementation. –  Feb 14 '12 at 22:37
  • Decompose! Decompose! Decompose! Yes, C++ allows you to write a single expression that does many magic things. But unless you understand what you do, why would you want this? – Konrad Rudolph Feb 14 '12 at 22:38
  • Oh.. your compare at the end.. First it is comparing the ponter, not the value, second the ++ after means it does the subtraction, then it does the increment.. It's not a good idea to have ++ on the same line as other things.. (unless you know how they work) – baash05 Feb 14 '12 at 23:56
  • actually, the original code is sort of fun.. it would change the source.. the only way I see it ending is if the first characters are equal. eventually it'd roll the value back to zero.. and exit.. No? – baash05 Feb 15 '12 at 06:02

6 Answers6

1

Because you don't stop when you've compared number characters.

There are several ways to do this, but I would recommend changing your loop condition to

while (*mystring1 && *mystring2 && *mystring1 == *mystring2 && number-- > 0)

Also remove

if ((*mystring1 == mystring1[z]) && (*mystring2 == mystring2[z])) 
{
    break;
}

Because, although it seems like that was your attempt at making it stop, it's coded wrong; you don't care if the characters are the same, you only care if you've compared number characters. Also you use && which makes the condition even more restrictive than it already was.

Also change

*mystring1++;
*mystring2++;

To

mystring1++; // or better, ++mystring1
mystring2++; // or better, ++mystring2

The * dereferences the pointer but you're not doing anything with it so it's pointless (pun intended).

You also can remove the ++ from these:

return (mystring1++ - mystring2++);

So it would be

return mystring1 - mystring2;

However, that is undefined behaviour when the two pointers point to different arrays (which they probably always will). You need to be doing something else. What? I don't know because I don't know what your function should return.

Seth Carnegie
  • 73,875
  • 22
  • 181
  • 249
0

You have no condition in your function that examines number, or z that you derive from it. What would make it stop?

Ned Batchelder
  • 364,293
  • 75
  • 561
  • 662
  • `if ((*mystring1 == mystring1[z]) && (*mystring2 == mystring2[z]))` seems *intended* to stop at position `z` (`number`-1). Yes, I know that check is wrong, but I cannot find the words to explain it in an answer. –  Feb 14 '12 at 22:39
0

Why don't you simply decrement number and break when it reaches 0 assuming the loop hasn't broken by that point

sizzzzlerz
  • 4,277
  • 3
  • 27
  • 35
0

You should update z on each iteration and then check if it reaches zero, try adding this to your code:

if (z == 0)
    break;
else
    z -= 1;

Also, that check you have there is really faulty, if it worked it could stop at an unwanted time, for example on the strings "abcdec" and "xxcddc", where number = 6, it would stop at 3, because the characters at those indexes are the same as those on index 6.

Re-read your code very thoroughly and make sure you really understand it before taking any of these answers into account.

wocoburguesa
  • 738
  • 1
  • 5
  • 7
0

This will walk until it finds a difference, or the end of the string. 

while(n > 0) {
    if(*str1 != *str2 || *str1 == '\0'){
          return *str1 - *str2;; //they're different, or we've reached the end.
    }
    ++str1; //until you understand how ++ works it's a good idea to leave them on their own line. 
    ++str2;
    --n;
}
return 0;// I originally had *str1 - *str2 here, but what if n came in as zero..

the problem with the z compare is it's a moving target. think of [] as a + sign.. mystring1[z] could be represented like this *(mystring1 + z) That means the line above ++mystring1; (as it should be) is moving the pointer and thus moving where z is looking..

It might help to think of pointers as address on a street.. when you ++ you move up a house.. Say z = 1.. and the house that mystring1 points at is yours, and z is your neighbor. add one to the house you're looking at, and mystring1 is now pointing at your neighbor, and z is pointing at his neighbor because z is still saying what your pointing at + 1.

baash05
  • 4,394
  • 11
  • 59
  • 97
  • you don't need to test both str's for 0.. if str2 is zero but str1 isn't, then the != would evaluate true. – baash05 Feb 14 '12 at 23:47
0

Thanks all...I fixed the error...added another condition to the while loop.

int i;
i=0;
z = number - 1;

while((*mystring1==*mystring2) && (*mystring1 !='\0') && (*mystring2 !='\0') && (i<z))

and then incrementing i till it comes out of this loop.

Apsara Sriram
  • 1