3
\$\begingroup\$

I've created this function that looks for sequences in a list. If [1,2,3] exists, it will print True. I've tested the code and it works fine.

def looking_patt(pattern,cursor,found):
    big_s = []
    for i in range(1,101):
        if i % 10 == 0:
            big_s.extend([1,2,3])
        else:
            big_s.append(i)
    # check control
    print(big_s)

    for i in big_s:
        if i == pattern[cursor]:
            cursor += 1
            if cursor == len(pattern):
                found.append(pattern)
                print(found)
                cursor = 0
        else:
            cursor = 0  

    print(len(found) > 0)    

Part of the script (the for i in big_s:) has been taken from an answer in this SO question.

How can it be improved?

\$\endgroup\$
5
\$\begingroup\$

I guess that the first part of your code is just to initialize the sample data.

Then here are a few things:

  • Separate initialization and main loop. I know this is just an exercise, but always doing it will help you keep the mindset.
  • If you only need to count something, just count it. Don't keep track of everything.
  • If you only need to check the existence of something, use a boolean value and return as soon as you've found it. Adding elements to a list so that at the end you can count them and return a boolean based on the count is definitely not an efficient way to do it.
  • What you call cursor is actually an integer, not an object, so the name index may be more appropriate
  • If you want to give the chance to start searching from a specific index, you may also want to have a default value for that parameter (0 looks like a good choice)
  • You don't need the third parameter, that's what you are going to return

This is my attempt at it:

big_s = []

def init_big_s():
    for i in range(1,101):
        if i % 10 == 0:
            big_s.extend([1,2,3])
        else:
            big_s.append(i)

def looking_patt(pattern, index=0):
    for i in big_s:
        if i == pattern[index]:
            index += 1
            if index == len(pattern):
                return True
        else:
            index = 0  
    return False

init_big_s()
print looking_patt([1, 2, 3])
\$\endgroup\$
  • \$\begingroup\$ Hi @chatterone, thanks for your feedback. ok for separating the functionS, this is what I did initially but I said to myself Hey why not put everything into one umbrella? Can you explain a bit more If you only need to check the existence of something, use a boolean value and return as soon as you've found it. Adding elements to a list so that at the end you can count them and return a boolean based on the count is definitely not an efficient way to do it. this part? \$\endgroup\$ – Andy K Jul 20 '16 at 9:35
  • 2
    \$\begingroup\$ @AndyK if you don't need to compute something, just don't do it :-) If the purpose of the function was to return the patterns that you found, you may have had a point. But if all you need is a check that the pattern exists, just return True as soon as the first pattern is found, no need to save the elements. \$\endgroup\$ – ChatterOne Jul 20 '16 at 10:50

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.