def disease_search(query)
treatment_results = Array.new
symptom_results = Array.new
image_results = Array.new
note_results = Array.new
disease_results = Disease.search("#{query} user_id:#{current_user.id}")
Treatment.search("#{query}").each do |treatment|
treatment_results.push(treatment.diseases)
end
Symptom.search("#{query}").each do |symptom|
symptom_results.push(symptom.disease)
end
Note.search("#{query}").each do |note|
note_results.push(note.disease)
end
Image.search("#{query}").each do |image|
image_results.push(image.disease)
end
search_results = (disease_results.concat(treatment_results).concat(symptom_results).concat(image_results).concat(note_results)).uniq
return search_results
end
Refactorings
No refactoring yet !
Oliver
April 11, 2011, April 11, 2011 16:16, permalink
As you're always returning diseases should't Disease.search("#{query} user_id:#{current_user.id}") return everything you need?
Adam Leonard
April 18, 2011, April 18, 2011 09:58, permalink
If you are using ActiveRecord you can just return the diseases and in your views do the following (Assuming you have your associations set up correctly). Most of what you are doing is very unnecessary work.
@diseases.treatments.each do |treatment| treatment.name end
Adam Leonard
April 18, 2011, April 18, 2011 09:58, permalink
If you are using ActiveRecord you can just return the diseases and in your views do the following (Assuming you have your associations set up correctly). Most of what you are doing is very unnecessary work.
@diseases.treatments.each do |treatment| treatment.name end
technicalpickles
April 25, 2011, April 25, 2011 20:59, permalink
What is backing `Disease.search`? If it's an indexing deal like sphinx or solr, you may be able to do some indexing shenanigans so you don't to search Treatment, Note, etc separately.
One thing to keep in mind is how many results any of these searches might return. If the dataset is sufficiently large, you might have a memory leak on your hand.
Anyways, here's some refactorings we can get in as is. Using `map` on the results will let you avoid having to allocate Array.new. Depending on what query is, you probably don't need to string interpolate it in most of the searches. We can end it with using
def disease_search(query)
disease_results = Disease.search("#{query} user_id:#{current_user.id}")
treatment_results = Treatment.search(query).map {|treatment| treatment.diseases }
symptom_results = Symptom.search(query).map {|symptom| symptom.disease }
note_results = Note.search(query).map {|note| note.disease }
image_search = Image.search(query).map {|image| image.disease }
disease_results.concat(treatment_results).concat(symptom_results).concat(image_results).concat(note_results).uniq
end
This is a really ugly method, but after looking at it all day trying to get it to work correctly, I'm afraid of breaking it.