Question about a Boolean Function

hi ,
i have written a program which recieves 2 strings and checks (is the second string "hasEndig" with 1 strings or not !)
for example : when we input strings : 1.Hello / 2.ello , it should return 1(TRUE) , and so on ...
this is the Code i've written ... i think my algorithm is correct but ... The program doesn't work Correctly ...!!
do u Know my algorithm ? if u didn't recognize , i can explain my algorithm ... thanks:)

Code:

---

#include <iostream>
using namespace std;

bool hasEnding(char str1[],int size1,char str2[],int size2){
    if (size2>size1)
        return false;

    for (int i=0 ; i<size1 ; i++)
        for (int j=0 ; j<size2 ; j++){
            while (str1[i]==str2[j] && j<size2){
                i++;
                j++;
                if (str1[i]!=str2[j])
                    return false;
            }
            return true;
           
        }
}

int main(){
    char string1[100],string2[100];
    cout << "Enter 1st String :\t";
    cin.get(string1,'0');
    cout << "Enter 2nd String:\t";
    cin>>ws;
    cin.get(string2,'0');

    int size1,size2;
    for (size1=0 ; size1<100 ; size1++){
        if (string1[size1]==0)
            break;
    }
    for (size2=0 ; size2<100 ; size2++){
        if (string2[size2]==0)
            break;
    }

    cout << "The 2nd string is repeated in 1st string ?? " <<endl;
    cout << "RESULT : \t " << hasEnding(string1,size1,string2,size2)<<endl;

    system("pause");
    return 0;
}

---

I think you need to change

Code:

---

while (str1[i]==str2[j] && j<size2)

---

To

Code:

---

while (str1[j]==str2[i] && j<size2)

---

Adjust the loop ending conditions as well, so your loops should be

Code:

---

for (int i=0 ; i<size2, i++)
  for (int j=0 ; j<size1; j++)

---

Right now you are iterating through the entire smaller string comparing it to the first letter of the larger string.

That change will get it to behave better but you still need to think about why

taster, ter
Would return false.

I've fixed the minor indentation problems you had. Makes it more readable.

You have a lot of things wrong in this code. Here are a few of them.

- You should really use std::string instead of char arrays. std::string is easier to work with and not in danger of overflow.
- cin.get will only read until it hits whitespace. If either of the strings you enter contains whitespace, you will get confusing results. You should use getline instead.
- The cin.get version that you're actually using is istream::get(char* buffer, streamsize buffer_size). Look at the overloads of cin.get and you will see why: there are two that take two arguments (as opposed to the two that take one and one that takes three), and one of those wants a streambuf as the first argument. So this doesn't do what you want at all - it appears you wanted to set the delimiter to something (probably a NUL character, but you actually pass an actual zero character), but what happens is that it interprets the character as the maximum string length, so it takes the numeric value of the character '0' (something around 80 I think) and uses that as the length. But the fix is to use getline anyway.
- This code is confusing:

Code:

---

    for (size1=0 ; size1<100 ; size1++){
        if (string1[size1]==0)
            break;
    }

---

I initially thought it doesn't do anything. Turns out it actually measures the string length. There's a library function for that, called strlen. Use it. (Better yet, use std::string, which knows its length.)

Moving on to the hasEnding function.

- Unless the search string is empty, neither of the for loops will ever repeat. The reason is that the inner for loop, once entered, will invariably end in a return statement: either the return false in the if statement in the while loop will be executed, ending the function early, or the return true immediately after the while loop will be. No way will you ever get a second iteration through the inner for loop. So the only situation where the outer for loop ever repeats execution is if the inner one isn't ever entered, which happens if size2 is zero. By the way, if that happens, the outer for loop will eventually terminate, and execution will fall off the end of your function without returning a meaningful value. Turn up your compiler's warnings, it should have told you about that. (If you're using GCC, pass -Wall.)
- The logic of your hasEnding function is just fundamentally flawed. Given the two nested for loops, it really appears that you didn't try to implement hasEnding ("Does string1 end with string2?") but isSubstring ("Does string1 contain string2 anywhere?"). hasEnding is far simpler: just calculate where in string1 string2 has to start (the endings of the strings have to line up) and do a simple comparison from there. isSubstring is much more complicated (you have to look at every possible place in string1 for string2), and you got it wrong.
- Consider what happens: first iteration of the outer loop, first iteration of the inner loop. You're comparing the first two characters of the two strings. If they are not equal, you return true. Why? If they are equal, you enter the while loop where you proceed to the next pair of characters. If they are not equal, you return false. Why? Also, in the while loop you mess with the indices of the for loops. What would happen if the for loop actually repeated? Well, you wouldn't simply get to the next character, as you wanted.
- For that matter, what is the purpose of the inner for loop? What is the purpose of the while loop?

I'll leave it at that.

At first sight, I'd say the the 'return' inside the 'while' loop is making the function end prematurely. Instead of returning false, you should 'break' from the 'while' loop, and avoid the other 'return' (either using a state variable or checking the condition of the while with an 'if').

Another option is changing all your loops for one that compares the characters backwards, starting at the end. This would be much more efficient.

Also, you should check the library functions for strings. A good one for this case is strstr(). At least, you should use strlen() for finding out the length of the strings ;-)

Quote:

---

Originally Posted by comocomocomo

[..]
Also, you should check the library functions for strings. A good one for this case is strstr(). At least, you should use strlen() for finding out the length of the strings ;-)

---

Oops. Sorry, I didn't realize that this is the C++ forum. You should use std::string and its methods length(), substr(), find()...

If you weren't getting a warning about "not all control paths return a value" then your compiler is not being told to use a high enough warning level.