CODEWITHSUNDEEP
ruby-on-railsactiverecordruby-on-rails-5rails-activerecord
Jason Shultz
Jason Shultz

Reputation: 970

Is there a cleaner way to return result of array from Rails ActiveRecord Method?

I have this method. I'm looping through each client and then checking to see if that client is eligible for terms or not. I feel that this is really inefficient. I believe that a method should return the value of the last action performed, but I still have to do this:

  def self.terms_qualifying
    qualifying_client = []
    Client.all.each do |client|
      qualifying_client << client if client.is_terms_eligible?
    end
    qualifying_client
  end

I know I can clean this up a little by doing this:

  def self.terms_qualifying
    qualifying_client = []
    return Client.all.each do |client|
      qualifying_client << client if client.is_terms_eligible?
    end
  end

But I feel like either I'm misunderstanding a key concept or I am just really not getting it. Can I make this more efficient?

Upvotes: 0

Views: 1068

Answers (2)

Smek
Smek

Reputation: 1208

Is is_terms_eligible a property persisted in the database as a boolean? In that case it would be possible to do something like this:

Client.where(is_terms_eligible: true)

That would be much more performant.

Upvotes: 1

Marcin Kołodziej
Marcin Kołodziej

Reputation: 5313

Instead of building an array yourself, you can simply loop over all your clients and choose the ones you're interested in with Array#select

def self.terms_qualifying
  Client.select(&:is_terms_eligible?)
end

Though if your is_terms_eligible? method can be moved to an SQL query, it could make everything even faster.

If you would still like to write your loop in a cleaner way, this would be the equivalent of above select:

Client.all.each_with_object([]) do |client, array|
  array << client if client.is_terms_eligible?
end

as Enumerable#each_with_object returns the object you've created inside the block.

Upvotes: 5

Related Questions