3
\$\begingroup\$

This is an implementation of contains, which returns a boolean indicating whether a pattern occurs in text.

I encountered this question while I was implementing find_all_index. For example, contains('abc', 'b') returns True and contains('abc', 'c') returns True.

I ran the following unit test cases, and it passed against all test cases, so the code seems to be working fine.

Implementation

Test

def contains(text, pattern):
    assert isinstance(text, str), 'text is not a string: {}'.format(text)
    assert isinstance(pattern, str), 'pattern is not a string: {}'.format(text)

    """create a pattern variable name """
    pattern = list(pattern)
    p_len = len(pattern)

    clean_text = list(filter(str.isalnum, text))
    t_len = len(clean_text)

    print("Searching contains")
    for i in range(t_len):
        # If the first letter matches, check the rest of string
        if clean_text[i] == pattern[0]:

            for j in range(p_len):
                if i+j >= t_len:
                    return False
                if pattern[j] != clean_text[i+j]:
                    break
                if j == p_len-1:
                    return True

    return False

def test_string_algorithms(text, pattern):
    found = contains(text, pattern)
    print('contains({!r}, {!r}) => {}'.format(text, pattern, found))

def main():
    test_string_algorithms(text, pattern)


if __name__ == '__main__':
    main()
\$\endgroup\$
3
  • 3
    \$\begingroup\$ Do you know about the in keyword? \$\endgroup\$
    – Peilonrayz
    Oct 31, 2017 at 16:30
  • \$\begingroup\$ I would be open to have more code review. \$\endgroup\$
    – NinjaG
    Nov 5, 2017 at 1:54
  • \$\begingroup\$ When Should I consider to start a bounty? \$\endgroup\$
    – NinjaG
    Nov 5, 2017 at 1:54

2 Answers 2

1
\$\begingroup\$

PEP8 and readability

  • You have a few great docstrings, there is no need for block-comments and docstrings.
  • In contains you have this assert isinstance(text, str) but don't use it.

Code changes

Your contains(text, pattern) could be a lot more simplified. Because contains checks whether a pattern occurs in text and returns a boolean indicating .

def contains(text, pattern):
    """Return a boolean indicating whether pattern occurs in text."""
    # If our pattern index is equal to our pattern length 
    # then we found a match and we can return True
    # If our current text character is not our pattern character 
    # and if we were checking for a pattern
    # If we have not found our pattern then we return False

    text_index = 0
    pattern_index = 0

    text_length = len(text)
    pattern_length = len(pattern)

    while text_index < text_length:  
        if text[text_index] == pattern[pattern_index]:
            pattern_index += 1
        elif pattern_index > 0:
            pattern_index = 0
            continue
        if pattern_index + 1 > pattern_length:
            return True
        text_index += 1
    return False
\$\endgroup\$
-2
\$\begingroup\$

Advice 1

if clean_text[i] == pattern[0]: You don't need this. Consider this version:

def contains_v2(text, pattern):
    for i in range(len(text)):
        do_continue = False

        for j in range(len(pattern)):
            if text[i + j] != pattern[j]:
                do_continue = True
                break

        if do_continue:
            continue
        return i;
    return -1

Advice 2

In your version, you do clean_text = list(filter(str.isalnum, text)). That is doing 2 things in a single function. I suggest you remove it, and if you need it in your version, you filter first, and only then pass it to your implementation.

\$\endgroup\$
0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.