1

I am beginner to programming and started to use Python as a means to learn it. I am currently working on a set of lines that use for loops () and while loops (). Currently I have this:

def missingDoor(trapdoor,roomwidth,roomheight,step):        
    safezone = []
    hazardflr = givenSteps(roomwidth,step,True)
    safetiles = []

    for i,m in enumerate(hazardflr):
    safetiles.append((m,step))
        while i < len(safetiles):
            nextSafe = safetiles[i]
            if knownSafe(roomwidth, roomheight, nextSafe[0], nextSafe[1]):
                if trapdoor[nextSafe[0]/roomwidth][nextSafe[0]%roomwidth] is "0":
                    if nextSafe[0] not in safezone:
                        safezone.append(nextSafe[0])
                    for e in givenSteps(roomwidth,nextSafe[0],True):
                        if knownSafe(roomwidth, roomheight, e, nextSafe[0]):
                            if trapdoor[e/roomwidth][e%roomwidth] is "0" and (e,nextSafe[0]) not in safetiles:
                                safetiles.append((e,nextSafe[0]))
            i += 1  
    return sorted(safezone)

then with a help from a community member, I was able to make it more efficient setting common variable nextSafe[0] to ns and just calling it from the top:

def missingDoor(trapdoor,roomwidth,roomheight,step):        
    safezone = []
    hazardflr = givenSteps(roomwidth,step,True)
    safetiles = []

    for i,m in enumerate(hazardflr):
    safetiles.append((m,step))
        while i < len(safetiles):
            nextSafe = safetiles[i]
            ns0 = nextSafe[0]
            if knownSafe(roomwidth, roomheight, ns0, nextSafe[1]):
                if trapdoor[ns0/roomwidth][ns0 % roomwidth] is "0":
                    if ns0 not in safezone:
                        safezone.append(ns0)
                    for e in givenSteps(roomwidth,ns0,True):
                        if knownSafe(roomwidth, roomheight, e, ns0):
                            if trapdoor[e/roomwidth][e%roomwidth] is "0" and (e, ns0) not in safetiles:
                                safetiles.append((e, ns0))
            i += 1  
    return sorted(safezone)

these all replace value efficiency, but is there any other way to make this more efficient and possibly line saving? the knownSafe(), givenSteps() function is from another code that works with this code to find possible safezones and knownsteps. But my question is just how to make this code more efficient because initially i started to use while loops() and found that for loops are better when given a known list. Been trying all sorts of things given that I am new to programming.

thank you in advance!!

3
  • 2
    Please make sure the indentation in your example is correct. At least the line after the for statement is in at a wrong indentation level. Commented Jul 11, 2014 at 13:54
  • 2
    Can you describe your algorithm? Commented Jul 11, 2014 at 13:58
  • 2
    If this code works (do you have test inputs and outputs to check that?) consider moving the question to codereview.stackexchange.com. Also, consider what you mean by efficiency - memory usage, processing speed, readability, ... Commented Jul 11, 2014 at 14:13

1 Answer 1

1

So .. You want Code Review ? Here ..

Anyways, I'll try to help you ..

Code

def missingDoor(trapdoor, roomwidth, roomheight, step):
    safezone, safetiles = [], []
    check = lambda a, b, c, l: knownSafe(roomwidth, roomheight, a, b) and trapdoor[a/roomwidth][a%roomwidth] == '0' and c not in l

    for i, m in enumerate(givenSteps(roomwidth, step, True)):
        safetiles.append((m, step))
        for _ in range(i, len(safetiles)):
            nextSafe = safetiles[i]
            ns0 = nextSafe[0]

            if check(ns0, nextSafe[1], ns0, safezone):
                safezone.append(ns0)
                safetiles.extend([ (e, ns0) for e in givenSteps(roomwidth,ns0,True) if check(e, ns0, (e, ns0), safetiles) ])
    return sorted(safezone)

Explanation

Line 1: function definition
Line 2: declaring the variables, this condensed style of declaring more than one variable on a line has nothing to do with efficiency, It's just a matter of style
Line 3: That's an important approach, because it shows how Functional Programming (one of the programming paradigms supported by python) can help in code clarity (again, no memory efficiency, just code efficiency, which will help in the long run) .. the lambda in a nutshell, is simply a condensed function that contains only one line of code, and returns it back, so no need for the return statement here, actually, there's more to the lambda than that, but that's basically why you'd like to implement one in a situation like that .. The goal of this specific one here, is to check the variables using the repeated check you ran every few lines I couldn't help but notice how messy it was! So this function exists to simply organize that .. The first and second parameters are self-explanatory, the third parameter is the one to check for its existence in the fourth parameter (which I suppose is a list) ..
Line 5: The loop start, nothing strange, except that I used the function's return value directly into the loop, because I thought storing it as a variable and using it only once would be a waste of RAM ..
Line 6: Unchanged ..
Line 7: I changed the while loop to a for loop, because I noticed the sole reason you used a while loop instead of a for loop is that the value of i changed every time the outer loop ran, so, why not provide a range ? That'd be more code and memory efficient
Line 8, 9: Unchanged ..
Line 11: We used the lambda, tadaaa ! Same call like a function ..
Line 12: Unchanged ..
Line 13: That technique here is called List Comprehension, that's a little bit advanced, but, in a nutshell, it creates a list that just gets the left-most value appended to each time the loop inside ran, any other conditions on the right are executed every time the loop ran, if returns true, the left-most value is appended, else, the loop continues over .. The list returned from all that, gets added to the safetiles list, note that: the extend method of a list simply appends all the elements in the argument list to the caller list, in this case: each of the elements of the list returned as a result of the list comprehension, simply gets appended to safetiles .. This isn't really an overkill as it may seem, because it actually clarifies code, and uses even less memory ..
Line 14: returns the safetiles list .. Mission Accomplished!

Hope this helps ;) Note that it's not good practice to check using "is '0'", better: "== '0'"

Sign up to request clarification or add additional context in comments.

5 Comments

in the line where it says: for i, m in enumerate(givenSteps(roomwidth, step, True)): this returns "TypeError: givenSteps() takes 4 arguments (3 given)"
Also Amr, is it possible to explain what is lambda a, b, c, l: knownSafe(roomwidth, roomheight, a, b) doing exactly
I explained everything ;)
@SteveZrg the TypeError you got is because you only provided three arguments for a function that requires four .. Note that the original code only provided three .. I didn't change the function call
so for that line what is the 4th argument? or am i overlooking something

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.