Leetcode 28 - Implement strStr(): question

Question

I am experiencing a bug in my submissions for Leetcode 28 that has thus far eluded me. My code works for most test cases but I am getting hung up on scenarios such as haystack = "mississippi", needle = "issip".

I have tried debugging and found that the entire haystack string is iterated through and it is returning -1 or not found. The substring length it is finding at each occurrence of 'i' is 4, 1, 1.

int strStr(string haystack, string needle) {
        if (needle.empty()) {
            return 0;
        }
        if (haystack.empty() && !needle.empty()) {
            return -1;
        }
        int i = 0, j = 0, ans = 0;
        for (i; i < haystack.length(); i++) {
            if (haystack[i] == needle[0]) {
                j = 0;
                ans = i;
                for (j; j < needle.length(); j++) {
                    /*
                    if (haystack[i++] == needle[j]) {
                        continue;
                    }
                    else {
                        break;
                    }
                    */
                    if (haystack[i++] != needle[j]) {
                        break;
                    }
                }
                if (j == needle.length()) {
                    return ans;
                }
            }
            if (j == needle.length()) {
            return ans;
            }
        }
        return -1;
    }

Input: "mississippi", "issip" Output: -1 (ans = 10, j = 1)

Unrelated to your issue but your second `if` is overcomplicated without a reason. You do not need to check again if `needle` is not empty. — Slava, Aug 06 '19 at 14:57
Maybe you should explain, why `haystack.find(needle)` is not an answer to your question? You probably have artificial constraints, but please list them. — nvoigt, Aug 06 '19 at 14:58
Do you see the side-effect in `haystack[i++]`? Do you see how it skips the regularity of the scanning in your `for (i;...` loop? Your assertion of "the entire haystack string is iterated through" is false. — Wyck, Aug 06 '19 at 14:59
Yes, the constraints are such that I would like to code the solution mostly by hand, excepting very simple methods such as empty() and length(). Also, I have not reformatted this problem so there will be redundancies; my main concern is why this specific bug is occurring. Thank you so much! — artifc3, Aug 06 '19 at 15:02

Vlad from Moscow · Answer 1 · 2019-08-06T15:37:52.150

The function has several drawbacks.

For starters it should be declared like

std::string::size_type strStr( const std::string &haystack, const std::string &needle );

and if the second string is not found in the first string the function should return std::string::npos as all similar member functions of the class std::string do.

The function parameters shell be of constant referenced types.

The condition in this if-statement

if (haystack.empty() && !needle.empty())

has a redundant operand. It could be rewritten like

if (haystack.empty())

This loop

for (i; i < haystack.length(); i++)

should stop its iterations when the size of the tail of the first string is less than the size of the second string.

in this if-statement

if (haystack[i++] != needle[j]) {

the variable i is incremented that results in incrementing the variable two times: one in this statement and the second time in the loop.

The second pair of these statements

        if (j == needle.length()) {
        return ans;

is redundant.

The function can be written the following way as it is shown in the demonstrative program.

#include <iostream>
#include <string>

std::string::size_type strStr( const std::string &haystack, const std::string &needle )
{
    if ( needle.empty() )
    {
        return 0;
    }
    else if ( haystack.empty() )
    {
        return -std::string::npos;
    }
    else
    {
        std::string::size_type ans = std::string::npos;

        auto n1 = haystack.length();
        auto n2 = needle.length();

        for ( std::string::size_type i = 0; ans == std::string::npos && i + n2 <= n1; i++ )
        {
            std::string::size_type j = 0;
            while ( j < n2 && haystack[i+j] == needle[j] ) j++;

            if ( j == n2 ) ans = i;
        }

        return ans;
    }
}

int main() 
{
    std::string haystack( "mississippi" );
    std::string needle( "issip" );

    std::cout << strStr( haystack, needle ) << '\n';

    return 0;
}

Its output is

4

score 2 · Accepted Answer · answered Aug 06 '19 at 15:00

2

The problem is that you modify i in

if (haystack[i++] != needle[j]) {

Thus preventing a second potential match from being explored. Try

if (haystack[i + j] != needle[j]) {

and fix any knock-on issues. I expect it to work as-is, though.

answered Aug 06 '19 at 15:00

Jeffrey

11,063
1
21
42

Ah, I do see! The i was incremented past the occurrence of the subsequent match. Thank you very much, this will help me a lot to not make the same mistake again. – artifc3 Aug 06 '19 at 15:08
1

Now, the thing to keep in mind, moving forward, is that this made your algorithm. _O(n*m)_ (product of haystack and needle length). Probably enough to pass this leetcode. But if you want to perfect it and learn more, check out Boyer-Moore algorithm, for _O(n+m)_ – Jeffrey Aug 06 '19 at 15:17
Thanks, it came to my attention that this was very inefficient (O(n*m)). I had gotten clued onto Knuth-Morris-Pratt, but I will check out Boyer-Moore, as well! – artifc3 Aug 06 '19 at 16:24

Leetcode 28 - Implement strStr(): question

2 Answers2