Find existing session

Question

I have this method in my rails app:

def find_or_create_session(fbid)
 @sessions = Session.all
 if @sessions.find_by facebook_id: fbid
 @session = @sessions.find_by facebook_id: fbid
 if ((Time.now - @session.last_exchange).fdiv(60)).to_i > 5
 @session = Session.create(facebook_id: fbid, context: {})
 end
 else
 @session = Session.create(facebook_id: fbid, context: {})
 end
 @session
end

It retrieves all the Sessions, and tries to find one with a given facebook_id. If there is one, it checks if it's a recent one (last_exchange was less than 5 minutes ago). If it's not, it creates a new one.

If it can't find a session for this facebook_id, it will create one.

This is pretty easy, but it seems to be a lot of if/else for such a straightforward thing.

Answer 1

Some notes:

• @session: A method called find_or_create_session should not change the state of instance variables.
• @sessions.find_by facebook_id: fbid is written twice. Use variables to avoid repetition. Also, the consensus in the Ruby community is to always write parens (except for DSL-like code).
• Time.now - @session.last_exchange: You can perform that check in the SQL query.
• Your method is basically doing find_or_create_session = existing_session || new_session, let the code mimic it.

I'd write:

def find_or_create_session(fbid, max_age: 5.minutes)
 Session.find_by(["facebook_id = ? AND last_exchange >= ?", fbid, max_age.ago]) ||
 Session.create(facebook_id: fbid, context: {})
end

Answer 2

I believe this is equivalent:

def find_or_create_session(fbid)
 if (@session = Session.find_by facebook_id: fbid) &&
 (Time.zone.now - @session.last_exchange).fdiv(60).to_i <= 5
 @session
 else
 @session = Session.create(facebook_id: fbid, context: {})
 end
end

Some notes:

• No need for an @sessions variable unless you'll be using it in the views
• Always use Time.zone.now instead of Time.zone