6 ms·

I've been doing Rails development for a decade. Concerns are probably the biggest code smell/anti-pattern I've ever seen in any application. It's used as a ba

I've been doing Rails development for a decade. Concerns are probably the biggest code smell/anti-pattern I've ever seen in any application. It's used as a bandaid to "break up" classes which do too much (but you really don't, you're just hiding the complexity), or it's way over done and everything is magic and almost incomprehensible and unmaintainable. Overall, just a total nightmare to deal with. Would not recommend, and I always try to steer people away from using concers during code reviews.

tomc1985 6y ago
Agreed. Concerns break large classes up to an innumerable amount of small files. It becomes so hard to keep track of simple interactions because they are spread across half a dozen files and/or contexts.
Axsuul 6y ago
What's the alternative to sharing logic across ActiveRecord models?
- seancoleman 6y ago
  I think a different way of looking at the problem is that ActiveRecord models really shouldn’t have much, if any logic. Instead, logic should get delegated to other types of object, such as services, decorators, query objects, etc. While “fat models” is certainly better than “fat controllers”, I’m afraid it lead the Rails community astray with large-scale apps that required more nuanced architecture.
- Fire-Dragon-DoL 6y ago
  Don't put the logic in activerecord models at all. Not even the data. Use activerecord uniquely as a querying mechanism (read or write), don't use relationships and don't put validations in there. Create objects (aka behavioral objects, aka servoce objects) for the logic and create entities (plain ruby objects) when you need to pass around the data. Yes, you are essentially eliminating the entirety of activerecord. After 10 years of rails, you realize that is the only safe way to use that library. By the way User User::SignUp Are related. The behavior doesn't need to be in the same object, the namespace takes care of that already.
  - swader999 6y ago
    That's a breath of fresh air to read.
  - megraf 6y ago
    You're correct on all fronts. Domain driven design, POROs- all of it. I bet you maintain something nice!
    - Fire-Dragon-DoL 6y ago
      Thanks, I really appreciate that. I do my best, but the sad reality is that I made an insanely large amount of mistakes in my development life, I didn't reach these conclusions without doing damage myself. I try to give back by training developers and making sure the business gain awareness so they don't have to experience the complete halt of feature development when the software becomes unsuited for changes.
  - e12e 6y ago
    > Use activerecord uniquely as a querying mechanism (read or write), don't use relationships and don't put validations in there. I'm sorry, but without relationships - why even use AR for querying? And how? Via connection.select/execute?
    - Fire-Dragon-DoL 6y ago
      It's hard to answer, in the sense that yes, you can use ActiveRecord relationships for the purpose of _building a query_. The point is, you should use them only for that. And it comes with the downsides of distancing you from SQL, which is not negligible. The problem is if you try to access relationships that you didn't load from the object, that shouldn't be done. It's the autoloading from the database that is the real problem. If you have enough discipline and enough authority to ensure that never happens within the software you are working on, you can. It's really culture problem. As soon as you pass around a `user`, someone will type `.posts` on it, or `.save`, suddenly your business logic depends on the database shape, rather than on contracts. What if your database shape is wrong? What if your database shape needs to change? The goal of good architecture is to be resilient to change, or to even _postpone the choice_ to a moment in time where you have more data to make the right one. The safest way is: query with activerecord, map to plain ruby objects and discard the activerecord ones immediately. This will also help you discover the entities you didn't know about, for example, if you have a table of users identified by email, that's the "User" table in Rails. However, let's say that you take only a subset of that table (with select), such as "Full Name" and "Email": this could represent something different. A Newsletter::Subscriber for example. And the newsletter subscriber entity can be used in the Newsletter::Weekly::Send object, as well as the Newsletter::Unsubscribe. If you realize that this should not have been in the users table in the first place (I have no idea myself), the cost of change is way lower than if you passed an activerecord object around. I hope this answers some of your questions. There is a lot to say on the topic. I'm happy to chat more on slack or an online videoconference session
- dasil003 6y ago
  If you are truly sharing logic then it might be okay, but typically I've seen concerns added as a way to organize code when ActiveRecord models get too big. At this point a lot of people reach for Concerns because it is right there in Rails waiting to be used, but ultimately it's just papering over the problem of bloated classes with too many responsibilities. I usually prefer to add some kind of service layer to handle complex manipulation and object lifecycle so that ActiveRecord just manages querying/persistence with most if not all business logic at a higher layer.
  - xipho 6y ago
    > it's just papering over the problem of bloated classes with too many responsibilities. If you're blindly coding away and using some feature just because it's there you've got bigger problems with how you go about programming, that's not special to Rails. I find concerns to be an intermediate mechanism to use in migrating the bloat out to some other abstract level. However, adding abstractions beyond MCV has it's own problems, each level means the developer has to understanding something more, one can't escape that. Pretending that one can fully anticipate the complexity of large project and model it perfectly via the way you organize your code is really only something done by the the beginning programmer. Concerns can be very tight, as you note, and of course any feature can be used poorly. I like concerns, we have many that haven't been touched, and lead to an ability to extremely rapidly spread functionality around a huge number of models (e.g. different annotation classes). I'm also guilty of using them in the intermediate manner above. In the long term, one sees these patterns, and doesn't fret about such things.
welearnednothng 6y ago
I empathize with everything you're saying here, but I do humbly disagree. I've experienced exactly what you're talking about, and it's a nightmare. Frankly, I think the Rails codebase itself even suffers from an over use of concerns that can make figuring out where functionality comes from very frustrating at times. That said, concerns are mixins, and mixins are, in the simplest of terms, more or less multiple inheritance. That's powerful stuff, and powerful stuff in the wrong hands (which includes every hand that'll touch it, not just the author!) will always get you in trouble. That could be the motto for Rails itself - powerful stuff, but in the wrong hands it'll get you into trouble! My first exposure to concerns in Rails was a awful codebase for a well known company with massive classes that someone went on a weekend bender and shrunk down to less massive classes by introducing dozens of concerns. It made the classes _appear_ smaller, but they weren't actually. In reality, it was just hiding away much of the complexity and horrible decision making that made the code so hard to reason about and maintain and took things from bad to worse as a result. That said, when you have a well architected code base to begin with (as in before you start introducing concerns) that has svelte classes with appropriate responsibilities, with shared logic broken out into "services" or whatnot, concerns can be a really nice way to share isolated functionality that can lead to cleaner classes without obfuscating what's going on. Quite the opposite - used well, they can tuck away (but not altogether hide) ancillary functionality allowing the meat of a method to be highlighted instead of drowned out by pre- and post-requisite behavior that might take up more LoC than the unique business logic of a given controller action thus obfuscating the functionality specific to that method. On my current and previous project, concerns worked out great for the teams (which included junior devs) but were used sparingly and typically introduced by experienced devs. Also, taking a quick glance, it seems like Controller concerns are favored far more, and often involve calls to class-level ActionController methods such as rescue_from, overriding protect_from_forgery defaults, settings layouts, before_action, helper_method, etc. - things that you can't just tuck into a service class or method that gets called from the controller and would otherwise require someone remembering the correct combinations of code to sprinkle into a controller or creating a controller to inherit from (which isn't practically different from concerns but can paint you into a corner). They're also never used for the business logic of an action, only supporting functionality (again, pre- and post- requisite functionality or introducing class-level helper methods that can be reused). All that being said, for a team that isn't confident in how to use concerns effectively, I'd also steer them away and guide them to consider alternative approaches. But at face value, I don't consider concerns an anti-pattern and have found that, when used appropriately, they can be a great tool in maintaining a clean codebase.

tomc1985 6y ago

Agreed. Concerns break large classes up to an innumerable amount of small files. It becomes so hard to keep track of simple interactions because they are spread across half a dozen files and/or contexts.

Axsuul 6y ago

What's the alternative to sharing logic across ActiveRecord models?

seancoleman 6y ago
I think a different way of looking at the problem is that ActiveRecord models really shouldn’t have much, if any logic. Instead, logic should get delegated to other types of object, such as services, decorators, query objects, etc. While “fat models” is certainly better than “fat controllers”, I’m afraid it lead the Rails community astray with large-scale apps that required more nuanced architecture.
Fire-Dragon-DoL 6y ago
Don't put the logic in activerecord models at all. Not even the data. Use activerecord uniquely as a querying mechanism (read or write), don't use relationships and don't put validations in there. Create objects (aka behavioral objects, aka servoce objects) for the logic and create entities (plain ruby objects) when you need to pass around the data. Yes, you are essentially eliminating the entirety of activerecord. After 10 years of rails, you realize that is the only safe way to use that library. By the way User User::SignUp Are related. The behavior doesn't need to be in the same object, the namespace takes care of that already.
- swader999 6y ago
  That's a breath of fresh air to read.
- megraf 6y ago
  You're correct on all fronts. Domain driven design, POROs- all of it. I bet you maintain something nice!
  - Fire-Dragon-DoL 6y ago
    Thanks, I really appreciate that. I do my best, but the sad reality is that I made an insanely large amount of mistakes in my development life, I didn't reach these conclusions without doing damage myself. I try to give back by training developers and making sure the business gain awareness so they don't have to experience the complete halt of feature development when the software becomes unsuited for changes.
- e12e 6y ago
  > Use activerecord uniquely as a querying mechanism (read or write), don't use relationships and don't put validations in there. I'm sorry, but without relationships - why even use AR for querying? And how? Via connection.select/execute?
  - Fire-Dragon-DoL 6y ago
    It's hard to answer, in the sense that yes, you can use ActiveRecord relationships for the purpose of _building a query_. The point is, you should use them only for that. And it comes with the downsides of distancing you from SQL, which is not negligible. The problem is if you try to access relationships that you didn't load from the object, that shouldn't be done. It's the autoloading from the database that is the real problem. If you have enough discipline and enough authority to ensure that never happens within the software you are working on, you can. It's really culture problem. As soon as you pass around a `user`, someone will type `.posts` on it, or `.save`, suddenly your business logic depends on the database shape, rather than on contracts. What if your database shape is wrong? What if your database shape needs to change? The goal of good architecture is to be resilient to change, or to even _postpone the choice_ to a moment in time where you have more data to make the right one. The safest way is: query with activerecord, map to plain ruby objects and discard the activerecord ones immediately. This will also help you discover the entities you didn't know about, for example, if you have a table of users identified by email, that's the "User" table in Rails. However, let's say that you take only a subset of that table (with select), such as "Full Name" and "Email": this could represent something different. A Newsletter::Subscriber for example. And the newsletter subscriber entity can be used in the Newsletter::Weekly::Send object, as well as the Newsletter::Unsubscribe. If you realize that this should not have been in the users table in the first place (I have no idea myself), the cost of change is way lower than if you passed an activerecord object around. I hope this answers some of your questions. There is a lot to say on the topic. I'm happy to chat more on slack or an online videoconference session
dasil003 6y ago
If you are truly sharing logic then it might be okay, but typically I've seen concerns added as a way to organize code when ActiveRecord models get too big. At this point a lot of people reach for Concerns because it is right there in Rails waiting to be used, but ultimately it's just papering over the problem of bloated classes with too many responsibilities. I usually prefer to add some kind of service layer to handle complex manipulation and object lifecycle so that ActiveRecord just manages querying/persistence with most if not all business logic at a higher layer.
- xipho 6y ago
  > it's just papering over the problem of bloated classes with too many responsibilities. If you're blindly coding away and using some feature just because it's there you've got bigger problems with how you go about programming, that's not special to Rails. I find concerns to be an intermediate mechanism to use in migrating the bloat out to some other abstract level. However, adding abstractions beyond MCV has it's own problems, each level means the developer has to understanding something more, one can't escape that. Pretending that one can fully anticipate the complexity of large project and model it perfectly via the way you organize your code is really only something done by the the beginning programmer. Concerns can be very tight, as you note, and of course any feature can be used poorly. I like concerns, we have many that haven't been touched, and lead to an ability to extremely rapidly spread functionality around a huge number of models (e.g. different annotation classes). I'm also guilty of using them in the intermediate manner above. In the long term, one sees these patterns, and doesn't fret about such things.

welearnednothng 6y ago

I empathize with everything you're saying here, but I do humbly disagree. I've experienced exactly what you're talking about, and it's a nightmare. Frankly, I think the Rails codebase itself even suffers from an over use of concerns that can make figuring out where functionality comes from very frustrating at times. That said, concerns are mixins, and mixins are, in the simplest of terms, more or less multiple inheritance. That's powerful stuff, and powerful stuff in the wrong hands (which includes every hand that'll touch it, not just the author!) will always get you in trouble. That could be the motto for Rails itself - powerful stuff, but in the wrong hands it'll get you into trouble! My first exposure to concerns in Rails was a awful codebase for a well known company with massive classes that someone went on a weekend bender and shrunk down to less massive classes by introducing dozens of concerns. It made the classes _appear_ smaller, but they weren't actually. In reality, it was just hiding away much of the complexity and horrible decision making that made the code so hard to reason about and maintain and took things from bad to worse as a result. That said, when you have a well architected code base to begin with (as in before you start introducing concerns) that has svelte classes with appropriate responsibilities, with shared logic broken out into "services" or whatnot, concerns can be a really nice way to share isolated functionality that can lead to cleaner classes without obfuscating what's going on. Quite the opposite - used well, they can tuck away (but not altogether hide) ancillary functionality allowing the meat of a method to be highlighted instead of drowned out by pre- and post-requisite behavior that might take up more LoC than the unique business logic of a given controller action thus obfuscating the functionality specific to that method. On my current and previous project, concerns worked out great for the teams (which included junior devs) but were used sparingly and typically introduced by experienced devs. Also, taking a quick glance, it seems like Controller concerns are favored far more, and often involve calls to class-level ActionController methods such as rescue_from, overriding protect_from_forgery defaults, settings layouts, before_action, helper_method, etc. - things that you can't just tuck into a service class or method that gets called from the controller and would otherwise require someone remembering the correct combinations of code to sprinkle into a controller or creating a controller to inherit from (which isn't practically different from concerns but can paint you into a corner). They're also never used for the business logic of an action, only supporting functionality (again, pre- and post- requisite functionality or introducing class-level helper methods that can be reused). All that being said, for a team that isn't confident in how to use concerns effectively, I'd also steer them away and guide them to consider alternative approaches. But at face value, I don't consider concerns an anti-pattern and have found that, when used appropriately, they can be a great tool in maintaining a clean codebase.