Reputation: 1074
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
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