Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
def get_all_friends(uid, client)
  friends = client.friendships.friends(uid: uid)
  total = friends.total_number
  get_friends(total, 0, uid, client)
end

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
end

get_all_friends(uid, observer.client).users.map do |info|
  user = User.find_or_create_by_uid_and_name(uid: info.id, name: info.name)
  follow(user)
  user
end

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?


Update:

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

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

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

  res = fetch_proc.call(count, cursor)
  left -= count
  next_cursor = res.next_cursor

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

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

1 Answer

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
add comment

Your Answer

 
discard

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.