views:

261

answers:

5

Hi, all. I'm a very, very new programmer. My language of choice at the moment is Python, and I feel like I have a decent feel for it. I'm just now starting to learn about recursion. (By the way, if anyone could recommend a good guide on this, please let me know!) Just so you all know, this question is very elementary, and the code I'm posting is horribly, horribly wrong.

Anyway, I'm trying to write a function that will get all the friends within a specified degree. If I pass it 0 as the degree, I just want myself. If I pass it 1, I want me and all my friends. 2, I want me, my friends, and all their friends, and so on.

I've tried quite a few different ways of doing this, but none work. I try to visualize how it should work in theory, and I can't quite get that either because I'm so inexperienced in this area. Maybe a kind soul here can show me all the ways in which this code fails and then explain how to do it properly and/or recommend a good guide on the subject. Here goes:

    def getFriends(self,degree,friendList):
        if degree == 0:
            friendList.append(self)
            return friendList
        else:
            friendList = friendList.append(self)
            for each in self.friends:
                each.getFriends(degree-1,friendList)

It doesn't work, and I know I've done stupid, stupid things. Someone please slap me and point me in the correct direction!

Thanks.

+1  A: 

You can move friendList.append(self) to the line before the if - you need it in both cases. You also don't need to assign the result to friendlist - it's a bug.

In your algorithm, you will likely to add the same people twice - if A is a friend of B and B is a friend of A. So, you need to keep a set of friends that you've processed already. Before processing, check this set and don't do anything if the person has been processed already.

Igor Krivokon
Thanks, that makes it a bit more concise. I've been changing the code around so much that I wasn't really thinking about it. Regarding duplicates, I was going to handle that after I get the thing to work at all. I know it'll be more efficient once I do that, but I'm trying to take it a step at a time.
BitingHobo
It would probably be a better idea to just add the objects to a treeset or something similiar; less objects to keep track of and all that.
Albinofrenchy
I ended up just adding a clause to only add self to the friendList if self isn't in the friendList already. Probably not that efficient, but I'm not worried about that.
BitingHobo
+12  A: 
friendList = friendList.append(self)

This sets friendList to None, unconditionally, as that's the invariable return value of any list's append method -- so, fix that weirdness first...!-)

Once you've fixed that, you still need to fix the function so that it always ends with return of something -- "falling off the end" returns None. E.g.:

def getFriends(self,degree, friendList):
    if degree == 0:
        friendList.append(self)
        return friendList
    else:
        friendList.append(self)
        for each in self.friends:
            each.getFriends(degree-1, friendList)
        return friendList

which can and clearly should be refactored to eliminate the duplication (DRY, Don't Repeat Yourself, is THE heart of programming...):

def getFriends(self,degree, friendList):
    friendList.append(self)
    if degree > 0:
        for each in self.friends:
            each.getFriends(degree-1, friendList)
    return friendList

PS: that (the alist=alist.append(...) issue) precisely how I got back in touch with my wife Anna in 2002 (we'd been not-quite-sweetheart friends many years before but had lost track of each other) -- she started studying Python, used exactly this erroneous construct, couldn't understand why it failed -- looked around the Python community, saw and recognized my name, mailed me asking about it... less than two years later we were married, and soon after she was the first woman member of the Python Software Foundation and my co-author in "Python Cookbook" 2nd ed. So, of course, I've got an incredible sweet spot for this specific Python error...;-).

Alex Martelli
Wow, congrats, man! Thanks for the tip, by the way. It's still returning None somewhere, but I'll see if I can weed that out.
BitingHobo
Once you've fixed this issue the function "runs off the end" (no return statement) and thus still returns None -- let me edit my answer to try and fix this!
Alex Martelli
Wow. Sir, I am in your debt. Thank you very much. Now I'm going to compare it to what I had to see what I did wrong. If you have any other advice, please elaborate further. And for what it's worth, you were my hero before I realized you were... uh... you. (First to answer my question, did it kindly regardless of how insignificant) Ha ha. :)
BitingHobo
Its a bit annoying that append and sort returns None. I personally think it should return the list again.
Unknown
@BitingHobo, thanks, but pls accept my answer if and only of you feel it's the most helpful one -- that's how StackOverflow works.@Unknown, mutators of built-ins always return None unless they have specific useful values to return (as, say, `.pop` does) -- they never `return self`, and at least the design's quite consistent in this way. No real reason to _support_ `x=x.append(y)` after all...!
Alex Martelli
@BitingHobo, in my first version I just fixed the two bugs -- erroneously assigning `append`'s return value, and missing a `return` at the end; in my second version I refactored... same behavior, better style (avoiding repetition) -- if you care about refactoring in Python maybe it's best if you ask a separate question so I and every other Python expert on SO can pitch in!
Alex Martelli
@Alex MartelliYour answer was most helpful, with a few others being specifically helpful on other things, but not for the overall problem. And when I say I'm new, I mean it- I don't even know what refactoring *is*. I've got a book on programming with Python, and it's pretty good, but I think a lot of this stuff is more about CS theory than just Python, so I'm left in the dark on it. Again, any tips you or anyone else can give me will be immensely appreciated.
BitingHobo
Alex, that is one heart-warming love story (in your post script) - thank you for sharing it! :)
micahwittman
+1  A: 

Is your identation correct? The body of the method should be indented relative to it's definition

artemb
It's correct, I guess I just didn't paste it correctly. Thanks, though. :)
BitingHobo
Fixed the indentation.
Matthew Flaschen
+1  A: 

There's no return statement in the else clause. So if degree != 0, this method will always return None. You want to append the result of each recursive getFriends call to your friendList, and then return friendList.

By the way, if you want to make this algorithm faster, there are well established methods for doing this with either graph algorithms or matrix manipulation. For example, if you represent friendship relationships with an adjacency matrix A, and you want to find all people who are within n degrees of separation of each other, you can compute B=A^n. If B[i][j] > 0, then i and j are within n degrees of separation of each other. Matrix multiplication is easy with a package like NumPy.

RexE
+1  A: 

(Sorry, I can't comment on Alex's answer... yet)

I don't really like the idea that getFriends returns a value that is never used. It works, for sure, but it looks a bit intriguing ;) Also, the first call to getFriends would be self.getFriends(degree, []) which is confusing: when getting a list of friends, why would you pass as an argument an empty list, right?

For clarity, I think that I would prefer this slightly different version, using the _getFriends helper function:

def getFriends(self, degree):
    friendList = []
    self._getFriends(degree, friendList)
    return friendList

def _getFriends(self, degree, friendList):
    friendList.append(self)
    if degree:
        for friend in self.friends:
            friend._getFriends(degree-1, friendList)
NicDumZ
I do like that better. Is that how overloading is done in Python? I can't tell you how new to this I am. ha ha. Thanks!
BitingHobo
Hello+ It has nothing to do with overloading :) getFriends and _getFriends are two completely different functions. _getFriends could be named _getFriendsHelper. The leading underscore means "this function is non-public/a helper function". Unlike Java, it still allows external access. Read http://mail.python.org/pipermail/python-list/2002-February/127959.html about it :)
NicDumZ