- 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)