Chris
Chris

Reputation: 1074

Optimizing Multiple Queries using ActiveRecord Rails

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!

Upvotes: 1

Views: 118

Answers (1)

fabOnReact
fabOnReact

Reputation: 5942

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.

read more about this in this post

If you have additional questions, just ask me.

Upvotes: 1

Related Questions