3
\$\begingroup\$

I wrote a simple function that finds a word in a string without use of the re module.

My function looks like this:

def find_in_string(string, word):
    Indices = ()
    wordlength = len(word)
    for i in range(0, len(string)):
        if string[i:i + wordlength] == word:
            Indices += (i,)
    return Indices

Is there any way to improve it?

\$\endgroup\$
  • \$\begingroup\$ How exactly do you define a "word"? Do you simply mean finding occurrences of a substring within a string? \$\endgroup\$ – 200_success May 1 at 1:21
4
\$\begingroup\$
  • I don't like the name of your function. Find what in string? Something like find_all_substring_indices() better describes what the function does
  • You aren't finding words, you're finding substrings. Your code would find the word "car" twice in "carcar" even though we would consider "carcar" a single word. You mention the re package. Presumably, you're talking about word boundaries. They behave a little differently (punctuation can be a word separator). But, since your original question doesn't behave that way, it is not a refactoring for me to suggest how to do that (nor is it on topic, "how do I replicate regex word boundaries?" is a question for StackOverflow)
  • Indices should be snake_case. Only class names are UpperCamelCase
  • Don't use a tuple for indices! Every time you append to it you have to create a new object because tuples are immutable. In fact, you are creating two: one for the (i,) and then another one when you do indices + (i,). Use a list: indices = [] then indices.append(i)! That's what they're meant for!
  • range(0, len(string)) can just be range(len(string))
  • string[i:i+wordlength] == word is the same thing as (and much less clear than) string[i:].beginswith(word)
  • All of this slicing is really inefficient, especially given there is also a string method that does what you want: index
  • You should unittest your code! I suspect you have a bug in that if your "word" is "FooFoo" it will be found twice in "Little bunny FooFooFoo" (once at the beginning of "FooFooFoo" and once three characters into "FooFoo"). Surely, words shouldn't be able to overlap like this.
  • Add """Docstrings.""" to document what your function does.

You should just be repeatedly using index until you read the end of the string. This is more efficient than doing slicing by yourself or sliding the substring character by character along your string. Stealing mostly from SO:

def find_all_substrings(string, sub):
    start = 0

    while True:
        try:
            start = string.index(sub, start)
        except ValueError:
            break

        yield start
        start += len(sub)

This is a generator, if you still want a tuple returned you can just build one up instead (but at a list first!):

def find_all_substrings(string, sub):
    indices = []
    start = 0

    try:
        while True:
            start = string.find(sub, start)
            indices.append(start)
            start += len(sub)
    except ValueError:
        pass

    return tuple(indices)
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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