I'm currently building an app and using namespaced service objects containing a single call method in each class.
Below is an actual method (I'm aware the ABC size is too large, I'm wanting to get it squared away before I break it apart into smaller chunks):
module Orders class CalculateNextStep include Services::Base def call(order) if order.next_step? order.next_step = Course.where(id: order.course_id).first.course_steps .where(position: 1).first.id else position = CourseStep.find(order.next_step).position order.next_step = Course.where(id: order.course_id).first.course_steps .where('course_steps.position > ?', position) .first.id end end end end
My question is should I move the queries into a query object? And should I just use raw PG queries versus AR finder methods?
When you say "optimize", what exactly do you mean? Are your current queries slow to perform, or would you just like to refactor them so they're more efficient to work with?
You don't really gain much in the terms of speed by doing raw sql vs an AR query (in most cases), and you can actually slow your queries down if you don't know what you're doing (in some cases). Also, raw sql is generally uglier / harder to read than AR methods so maintainability can become an issue later.
As for refactoring to be more efficient to work with, will these queries only ever be used when your app calls on this service object or are these queries used in multiple places? If they're only ever used like this here, then I'd question what's the benefit of abstracting it? Do you gain anything by putting that snippet of code in another file? If not, then just reorganize your code in this file so it's easy to come back to a few weeks / months / years later and call it a day!
Well hey right back at you Jacob!
Sorry for not explaining better. I'd like to refactor them to be more efficient to work with and speed them up as much as possible.
The calls will be used in multiple places. If a call is only going to be used in one place, I just place it in the model class.
I appreciate the input!
Sweet, then yeah I'd just clean up the queries some and ensure you have proper indexes on your tables. I'm far from an expert of DB query optimization but I really don't think you're going to gain much by moving away from what AR provides you, and so I'd spend my energy elsewhere in the project.
As for abstracting it out, really all this boils down to is maintainability. I don't believe there's really any measurable performance increase by having all your queries for that model in the model file vs abstracted out into 50 files. So I'd just do what makes the most sense for how your application functions. If you're ever bored, read up on design patterns, lot of good info there!
If you have more than one result but always take
first and you don't
sort not only that it might be inconsistent , you're pulling data you don't need. Add
limit(1) on the query.
Since you're only looking for
id anyhow , don't load the entire
pluck to to improve your memory and runtime performance.
Can you add a
course_id indexed foreign key to
CourseStep model? Than you will be able to query
CourseStep.where( course_id: order.course_id , position: 1 ).limit(1).take.pluck(:id).first
select was mentioned when you still want to fecth
pluck returns an array.
If you want to limit the size , use
limit as I suggested.
If you want to minimze your memory footstamp
Model.where( condition ).select(:id) should be smaller thatn
Model.where( condition ) while
Model.where( condition ).pluck will be the smallest (
pluck also receive attribute names like
Model.where( condition ).pluck(:id) ).
Note in the link you provided:
Almost always, you'll want to use where **rather** than select. So, when might you use select? Well you'd use it (**often in conjunction with where**) when you want to filter a relatively small number of records based on some ruby conditions that would be reasonably hard to translate to SQL.
I never said you should use
select instead of
where but ONLY in conjunction , I actually urge to always use
limit to avoid corner cases blowing up your machine memory.
don't load the entire ActiveRecord use select and pluck to to improve your memory and runtime performance.
That's the part that I'm referring to. If you have experience with queries then you already know this, but to newer developers the fact that
select still loads every object into memory is why I said to use caution. I'm certinally not saying to never use it, just be cautious of how you're using it, it's an iceberg function.
It's not necessarily a single query that would illustrate the problem, it's how it's used that's the problem. That's why I say it's not something a more experienced developer may get caught by, but to a new developer, the consequences aren't so obvious.
Off the top of my head, let's say you have a process that generates a lot of stats for a large group of records. Depending on the calculation being run, you may need to do some additional filtering from the initial group of objects you passed to the generator before passing it through to that particular calculation.
In this instance, a new developer may be inclined to use
select to handle this filtering, which may be a perfectly feasible solution if you know that there will never be a lot of records or you can safely use
limit, however, you may not always be able to use
limit or guarantee that there won't be a lot of objects to process. So care should be given as you're manipulating your data that you don't have any unexpected bottlenecks.
I'm not saying what you said was wrong in any way, just trying to add to the knowledge pool from personal experiences. :)
Join 22,346+ developers who get early access to new screencasts, articles, guides, updates, and more.