Created
June 20, 2019 00:51
-
-
Save Shpigford/35762ffedda58dbd1cf1ea86a42399be to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
def filtered_tracks(current_user) | |
tracks = current_user.tracks.limit(500) | |
rules = filters['rules'] | |
if rules.find {|h| h['id'] == 'track_name'}.present? | |
track_name = rules.find {|h| h['id'] == 'track_name'}['value'] | |
tracks = tracks.where('tracks.name ILIKE ?', '%' + track_name + '%') | |
end | |
if rules.find {|h| h['id'] == 'artist_name'}.present? | |
artist_name = rules.find {|h| h['id'] == 'artist_name'}['value'] | |
tracks = tracks.joins(:artist).where('artists.name ILIKE ?', '%' + artist_name + '%') | |
end | |
tracks | |
end |
@dummied: This is essentially outputting a query builder. So each tracks
call tacks on a new where
query, essentially.
Doing these bits over and over (there will be dozens of these if statements), feels...not correct.
if rules.find {|h| h['id'] == 'track_name'}.present?
track_name = rules.find {|h| h['id'] == 'track_name'}['value']
def filtered_tracks(current_user)
tracks = current_user.tracks.limit(500)
rules = filters['rules']
rules.find {|h| h['id'] == 'track_name'}.try do |rule|
tracks = tracks.where('tracks.name ILIKE ?', '%' + rule['value'] + '%')
end
rules.find {|h| h['id'] == 'artist_name'}.try do |rule|
tracks = tracks.joins(:artist).where('artists.name ILIKE ?', '%' + rule['value'] + '%')
end
tracks
end
Enumerable#find
returns the matching result (or nil
). Object#try
runs the code in the block (yielding the object to the block), unless the object is nil
(in which case it does nothing).
[1] pry(main)> rules = [ {'id' => "track_name", 'value' => "a"}, {'id' => "artist_name", 'value' => "b" }]
=> [{"id"=>"track_name", "value"=>"a"}, {"id"=>"artist_name", "value"=>"b"}]
[2] pry(main)> rules.find{|h| h["id"] == "track_name"}.try{|rule| "Add Rule: " +rule["value"] }
=> "Add Rule: a"
[3] pry(main)> rules.find{|h| h["id"] == "beats_per_minute"}.try{|rule| "Add Rule: " +rule["value"] }
=> nil
You could also consider extracting a method like:
def find_rule(rules, rule_name)
rules.find{|r| r['id'] == rule_name}
end
if you don't want to keep typing r["id"] == "blah"
def filtered_tracks(current_user)
tracks = current_user.tracks.limit(500)
rules = filters['rules']
find_rule(rules, 'track_name').try do |rule|
tracks = tracks.where('tracks.name ILIKE ?', '%' + rule['value'] + '%')
end
find_rule(rules, 'artist_name').try do |rule|
tracks = tracks.joins(:artist).where('artists.name ILIKE ?', '%' + rule['value'] + '%')
end
tracks
end
def find_rule(rules, rule_name)
rules.find{|r| r['id'] == rule_name}
end
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Which should take precedence? Right now, artist rules will override track name rules.
Assuming you want that to continue, maybe something like this (whether its better or not is your call):
This is all roughly pseudo-code. There are other nips and tucks you could do, too (like going with more of a
return ... if ...
pattern if that floats your boat).Your mileage/taste in code may vary.