This is fine, but there's a technique called "memoization" which is slightly more efficient
@current_user ||= User.find(session[:user_id]) if session[:user_id]
The "||=" basically means "assign only if not already assigned" and so reduces the number of database retrievals. We store the result in an instance variable because that will be retained throughout the time the current controller object is running.