Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top
def get_all_friends(uid, client)
  friends = client.friendships.friends(uid: uid)
  total = friends.total_number
  get_friends(total, 0, uid, client)

def get_friends(total, cursor, uid, client)
  left = total
  count = [200, left].min

  friends = client.friendships.friends(uid: uid, count: count, cursor: cursor)
  left -= count
  next_cursor = friends.next_cursor

  return friends if (left == 0 || next_cursor == 0)

  more_firends = get_friends(left, next_cursor, uid, client)
  more_firends.users << friends.users
  return more_firends

get_all_friends(uid, observer.client) do |info|
  user = User.find_or_create_by_uid_and_name(uid:, name:

This code is written in a functional style. Maybe there is a more natural way to structure it with Block.

And how can I reuse the code when client.friendships.friends(uid: uid, count: count, cursor: cursor) differs?


I reconstruct the code. It now looks more clean and general.

def get_all_friends(client, uid)
  fetch_all do |count, cursor|
    client.friendships.friends(uid: uid, count: count, cursor: cursor)

def fetch_all(&fetch_proc)
  res =, 0)
  total = res.total_number
  return res if total <= 10
  fetch(total, 0, &fetch_proc)

def fetch(total, cursor, &fetch_proc)
  left = total
  count = [200, left].min

  res =, cursor)
  left -= count
  next_cursor = res.next_cursor

  if (left == 0 || next_cursor == 0)
    return res

  more_res = fetch(left, next_cursor, &fetch_proc)
  more_res.users << res.users
share|improve this question

It looks pretty good to me. Some notes:

  • I'd avoid return on the last expression
  • I'd avoid the "-=" (not very functional)
  • I'd avoid inline returns (write a full-fledged conditional instead)
  • I have my doubts about this more_friends.users <<, is this a side-effect you are doing behind the caller's back?

As a general note, Ruby is an OOP language, you can strive for functional style (I do whenever possible), but still, use also an OOP structure or you'll end up with weird undiomatic code.

share|improve this answer
I do want the side-effect of more_friends.users <<. – gfreezy Dec 15 '12 at 14:11
+1 re inline returns. I used to use them a lot but now I prefer an if/else structure. – Eric Walker Dec 15 '12 at 21:52

Your Answer


By posting your answer, you agree to the privacy policy and terms of service.

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