Optimizing Multiple Queries using ActiveRecord Rails

Question

I have a method which currently functions correctly but I'd like to know if there's a better way of doing it.

There's a queue of items (songs) where the index increases and it updates accordingly.

This is my current code:

def self.get_next_schedule_track(schedule)
  begin
    # get current position
    current_position = schedule.current_position
    current_schedule_track = scheduled_track_at_position(schedule, current_position)

    # check if next position is valid, otherwise pick random track
    next_position = current_position + 1
    next_schedule_track = scheduled_track_at_position(schedule, next_position)

    if next_schedule_track.nil?
      random_track  = Track.where.not(id: current_schedule_track.track.id).order("RANDOM()").first
      ScheduleTrack.create(track: random_track, schedule: schedule)
      next_schedule_track = scheduled_track_at_position(schedule, next_position)
    end

    # advance current_position
    schedule.current_position = next_position
    schedule.save

    # update state of schedule tracks
    current_schedule_track.update(state: 'playing') unless current_schedule_track.nil?
    next_schedule_track.update(state: 'next') unless next_schedule_track.nil?

    # if previous track set state to played
    previous_schedule_track = scheduled_track_at_position(schedule, current_position - 1)
    previous_schedule_track.update(state: 'played') unless previous_schedule_track.nil?

    # broadcast new current position
    ActionCable.server.broadcast "schedule_channel", { title: 'next position', body: next_schedule_track.id }

    # return next_track
    next_track = next_schedule_track.track
    Rails.logger.debug "next_track = #{next_track.title}"
    next_schedule_track
  rescue Exception => e
    Rails.logger.error ("[ScheduleService.get_next_schedule_track] there was an error = #{e}")
    nil
  end
end

This does work and is easy to read, but it also makes several SQL requests so I wonder if there is a better approach?

Here is the log showing all of the requests.

 starting get next schedule
   ScheduleTrack Load (0.6ms)  SELECT "schedule_tracks".* FROM "schedule_tracks" WHERE "schedule_tracks"."schedule_id" = $1 AND "schedule_tracks"."position" = $2 ORDER BY "schedule_tracks"."position" ASC  [["schedule_id", 22], ["position", 2077]]
   ScheduleTrack Load (0.8ms)  SELECT "schedule_tracks".* FROM "schedule_tracks" WHERE "schedule_tracks"."schedule_id" = $1 AND "schedule_tracks"."position" = $2 ORDER BY "schedule_tracks"."position" ASC  [["schedule_id", 22], ["position", 2078]]
   Track Load (0.3ms)  SELECT  "tracks".* FROM "tracks" WHERE "tracks"."id" = $1 LIMIT $2  [["id", 74020682], ["LIMIT", 1]]
   Track Load (0.4ms)  SELECT  "tracks".* FROM "tracks" WHERE ("tracks"."id" != $1) ORDER BY RANDOM() LIMIT $2  [["id", 74020682], ["LIMIT", 1]]
    (0.1ms)  BEGIN
   ScheduleTrack Load (2.6ms)  SELECT  "schedule_tracks".* FROM "schedule_tracks" WHERE "schedule_tracks"."schedule_id" = $1 AND ("schedule_tracks"."position" IS NOT NULL) ORDER BY "schedule_tracks"."position" DESC LIMIT $2  [["schedule_id", 22], ["LIMIT", 1]]
   SQL (0.5ms)  INSERT INTO "schedule_tracks" ("position", "track_id", "schedule_id", "created_at", "updated_at") VALUES ($1, $2, $3, $4, $5) RETURNING "id"  [["position", 2078], ["track_id", 416171102], ["schedule_id", 22], ["created_at", "2017-10-30 12:04:22.470182"], ["updated_at", "2017-10-30 12:04:22.470182"]]
 [ActionCable] Broadcasting to schedule_channel: {:title=>"created track", :body=>4984}
    (0.6ms)  COMMIT
   ScheduleTrack Load (0.8ms)  SELECT "schedule_tracks".* FROM "schedule_tracks" WHERE "schedule_tracks"."schedule_id" = $1 AND "schedule_tracks"."position" = $2 ORDER BY "schedule_tracks"."position" ASC  [["schedule_id", 22], ["position", 2078]]
    (0.2ms)  BEGIN
   SQL (0.3ms)  UPDATE "schedules" SET "current_position" = $1, "updated_at" = $2 WHERE "schedules"."id" = $3  [["current_position", 2078], ["updated_at", "2017-10-30 12:04:22.482505"], ["id", 22]]
    (0.4ms)  COMMIT
    (0.2ms)  BEGIN
   SQL (0.5ms)  UPDATE "schedule_tracks" SET "state" = $1, "updated_at" = $2 WHERE "schedule_tracks"."id" = $3  [["state", "playing"], ["updated_at", "2017-10-30 12:04:22.487240"], ["id", 4983]]
    (0.5ms)  SELECT COUNT(*) FROM "schedule_tracks" WHERE "schedule_tracks"."schedule_id" = $1 AND ("schedule_tracks"."position" = 2077)  [["schedule_id", 22]]
    (0.6ms)  COMMIT
    (0.5ms)  BEGIN
   SQL (1.2ms)  UPDATE "schedule_tracks" SET "state" = $1, "updated_at" = $2 WHERE "schedule_tracks"."id" = $3  [["state", "next"], ["updated_at", "2017-10-30 12:04:22.504735"], ["id", 4984]]
    (2.0ms)  SELECT COUNT(*) FROM "schedule_tracks" WHERE "schedule_tracks"."schedule_id" = $1 AND ("schedule_tracks"."position" = 2078)  [["schedule_id", 22]]
    (0.9ms)  COMMIT
   ScheduleTrack Load (0.5ms)  SELECT "schedule_tracks".* FROM "schedule_tracks" WHERE "schedule_tracks"."schedule_id" = $1 AND "schedule_tracks"."position" = $2 ORDER BY "schedule_tracks"."position" ASC  [["schedule_id", 22], ["position", 2076]]
    (0.2ms)  BEGIN
   SQL (0.5ms)  UPDATE "schedule_tracks" SET "state" = $1, "updated_at" = $2 WHERE "schedule_tracks"."id" = $3  [["state", "played"], ["updated_at", "2017-10-30 12:04:22.519705"], ["id", 4982]]
    (0.6ms)  SELECT COUNT(*) FROM "schedule_tracks" WHERE "schedule_tracks"."schedule_id" = $1 AND ("schedule_tracks"."position" = 2076)  [["schedule_id", 22]]
    (0.6ms)  COMMIT
 [ActionCable] Broadcasting to schedule_channel: {:title=>"next position", :body=>4984}
   Track Load (0.4ms)  SELECT  "tracks".* FROM "tracks" WHERE "tracks"."id" = $1 LIMIT $2  [["id", 416171102], ["LIMIT", 1]]
 next_track = New York

Any advice on what the best approach is would be much appreciated—is there a better way, or is this a good solution.

Many thanks!

It looks really overcomplicated and that method is way to long - start by breaking it up into separate methods which perform a single task. You should also include the models in the question as there probally is a much simpler overall solution — max
– max, Commented Oct 30, 2017 at 12:47

fabOnReact · Accepted Answer · 2017-10-30 13:12:42Z

1

the best to write dry code in rails is doing the following:

1. Using the MVC Architecture

This requires separating code between your Model, View and Controller. The basic concept is creating your own custom methods in the Model Class so that you can call that method several times and save a lot of lines in your code.

For example,

# check if next position is valid, otherwise pick random track
next_position = current_position + 1
next_schedule_track = scheduled_track_at_position(schedule, next_position)

Move this code to the Position.rb model

def check_validity
    next_position = self + 1
    // perform the rest of your validity checks in hear
    // I don't know how to get hear the scheduled_track_at_position
end

while for the rest, in this I find things pertinent to the Track model, to the SceduleTrack Model and to other models... I believe you call scheduled_track_at_position from hear because it is a method in the same model, it looks disorganized to me like that.

if next_schedule_track.nil?
  random_track  = Track.where.not(id: current_schedule_track.track.id).order("RANDOM()").first
  ScheduleTrack.create(track: random_track, schedule: schedule)
  next_schedule_track = scheduled_track_at_position(schedule, next_position)
end

in Track.rb create a random_track class method

def self.random_track(schedule)
    self.where.not(id: schedule.track.id).order("RANDOM()").first
end

you can call the method from Song.rb with

Track.random_track(current_schedule_track.track.id)

but I can DRY this by creating a method in my Song.rb

def track_id
    current_schedule_track.track.id
end

so maybe this could work out

Track.random_track(track_id)

2. Using Inheritance and Mixins

The second way to dry up even more your code, is not only moving the code from your Song.rb module to the appropriate method in the Track.rb or in the other Models, but if a method should be used by more then one on this Models, but the Model already have a parent, then create a Module and either extend or include that Module methods in your Class.

3 Comments

Chris Over a year ago

Thanks very much indeed, that makes a lot of sense and it is going to take me some time to try to follow these instructions.

fabOnReact Over a year ago

@Chris actually I am also trying to improve my code. My advise is to check out this community called codereview.stackexchange.com and also read about attr_accessor, inheritance, lambda, proc, yield, modules etc...

Chris Over a year ago

I didn't know that existed! Thank you so much, I've used many of those concepts you list before but struggle to sometimes know which approach to adopt in specific situations. Your pointers have been very useful to me. Thanks again!

Collectives™ on Stack Overflow

Optimizing Multiple Queries using ActiveRecord Rails

1 Answer 1

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related