1

I have this loop:

  stations = Station.where(...)
  stations.all.each do |s|
    if s.city_id == city.id
      show_stations << s
    end
  end

This works well, but because of looping the all the data, I think it's kinda slow. I've tried to rewrite it with using select, like this:

show_stations << stations.select { |station| station.city_id == city.id}

But the amount of saved data into show_stations is different compared to the each version and then, the data are in different format (array/object).

Is there any better/faster way to rewrite the loop version?

Piccolo
  • 1,612
  • 4
  • 22
  • 38
user984621
  • 46,344
  • 73
  • 224
  • 412
  • where does `city` come from? The more logic you can put into the `where` clause, the faster the code will be because the returned dataset will be smaller. – the Tin Man Sep 04 '14 at 23:21

4 Answers4

1

The fastest version of this maybe the built-in rails ActiveRecord method for finding associated objects.

So provided your Station model contains this:

class Station < ActiveRecord::Base
  belongs_to :city

And your City model contains this:

class City < ActiveRecord::Base
  has_many :stations

Then rails automatically generates the method city.stations which automatically fetches the stations which contain that city's id from the database. It should be pretty optimized.
If you want to make it even faster then you can add add_index :stations, :city_id to your table in a migration and it will retrieve faster. Note that this only saves time when you have a lot of stations to search through.

If you need to make it an array you can just convert it after with city.stations.to_a. And if you wanted to narrow it further, just use the select method and add the conditions that you wanted to previously add in your Station.where(...) statement.
(e.g. city.stations.to_a.select { |item| your_filter })

cm92
  • 191
  • 1
  • 6
1

You should also cache the query results like

    stations ||= Station.where("your where").where(:city_id => city.id)
Minski
  • 21
  • 2
0

Station appears to be an active record model. If that is the case, and you don't need all the stations, you can add the city.id filter to your where statement.

The issue you're having now is that you're adding the array returned from select as the last item of show_stations. If you want show_stations to only contain stations that match city.id then use show_stations = ... rather than show_stations << .... If you want show_stations to contain what it already contains plus the stations that match city.id then use show_stations + stations.select { |station| station.city_id == city.id }. (There are a number of other approaches for adding two arrays together.)

Community
  • 1
  • 1
Jay Mitchell
  • 1,230
  • 11
  • 14
0

Maybe you need to include into the where clause the city parameter:

stations = Station.where("your where").where(:city_id => city.id)

or the same

stations = Station.where("your where").where('city_id = ?', city.id)
rogelio
  • 1,541
  • 15
  • 19