7 ms·
Ensuring that the Ticket is valid is obviously a domain model concern. Policy objects can be a fine idea when they're swappable and you need to allow for multip
by dhh 13y ago
Ensuring that the Ticket is valid is obviously a domain model concern. Policy objects can be a fine idea when they're swappable and you need to allow for multiple different policies. This is not one of those cases.
Here's a much simpler approach that keeps the validation logic in the domain model and uses features that Rails has had since the dawn of the framework: https://gist.github.com/dhh/9672827 https://gist.github.com/dhh/9672827
- dhh 13y agoOn a separate note, I feel like this article series might better be titled "The Missing Parts of Our Knowledge of Basic Rails Features". Reinventing basic features doesn't make your Rails deployment "advanced", it just makes it convoluted. There's no bonus prize for introducing fancy patterns to situations that do not call for them. Further more, here's the definition of the Active Record pattern, as described by Martin Fowler: "An object that wraps a row in a database table or view, encapsulates the database access, and adds domain logic on that data". The key part is that last sentence. So a quote like what follows simply misunderstands the purpose the Active Record pattern: "A recurring theme in these posts is that ActiveRecord models should be very simple interfaces to a datastore – the User model should be responsible for validating and persisting attributes of a user, like name and date-of-birth, and not much else." See the example diagram (which I actually drew for Martin back before even starting Rails!): http://www.martinfowler.com/eaaCatalog/activeRecord.html http://www.martinfowler.com/eaaCatalog/activeRecord.html -- it includes domain logic methods like "getExemption", "isFlaggedForAudit", and so forth. Exactly like the pattern describes. You're not a beautiful and unique snowflake.
- pothibo 13y ago>> On a separate note, I feel like this article series might better be titled "The Missing Parts of Our Knowledge of Basic Rails Features". Agreed on so many level. I see so many new gems that does the exact same thing as what rails do by default. I've spent some times browsing Rails source. It took me a while and as a side effect, the number of gems I am using now is a fraction of what I used when I started. You can't build libraries and abstraction on top of Rails if you don't understand rails to begin with.
- tomblomfield 13y agoThanks for taking a look - I certainly respect your viewpoint. I guess where we diverge is the amount of domain logic which lives on an ActiveRecord model. My experience is relatively limited, but in 4 or so years of professional Rails development, across many different codebases, the overwhelming majority of problems have been caused by bloated models with too many responsibilities. You clearly have more experience with Rails, but your solution always seems to be "If you were smarter, you wouldn't have these problems with my framework". I've seen this problem recur again and again across multiple teams & codebases. We're seeking to address that problem with these concepts that have been successfully used in other frameworks & languages.
- dhh 13y agoYes, when stumbling across bad code, the first instinct should be: how can I make this simpler. Not how can I wrap this bad code in more convoluted patterns. Don't use a big word when a small one will do. Additionally, I find that the key problem with bloated models stems from missing domain models (often has-many-through models). Not from moving the logic of the existing models into more noun classes. In your example here, the purpose of the ticket is to tie a user to a grouper: that's exactly the place to put logic that governs that connection! Finally, what gets my goat is this notion that these patterns are necessitated by "advanced deployments". As it was some law of nature that when your app hits a certain size, you have to introduce this litany of misfit patterns. That's like arguing that if your book is longer than 300 pages, it must also use really big words and complex sentence structures. What? The solution to large applications is to double down on simplicity, not give up on it. It is even more important to get the basics right. Execute them beautifully before even contemplating to freewheel from there.
- nthj 13y ago> Additionally, I find that the key problem with bloated models stems from missing domain models (often has-many-through models). Not from moving the logic of the existing models into more noun classes. When I first started learning Rails 4 years ago, I watched a video of a talk you gave where you refactored a kludgy piece of code to use an additional resource/model. The code unraveled before our eyes. I still remember that as a Neo moment.
- mnarayan01 13y agoI (more or less) agree with the sentiment expressed by "The Missing Parts of Our Knowledge of Basic Rails Features". That said, looking at your example, grouper_cant_be_full would not allow me to provide their desired functionality unless I already knew how to do it. Simply passing a symbol rather than a string there, along with a fairly short comment about what to do with it (i.e. in the controller and config/locales) would easily help people expand their knowledge. In short, you might be right in the above sentiment, but posts like these make hard for me to fault people like the author. Edit: I should probably note that I _don't_ think you should be "required" to help out like this at all. Simply developing/sharing Rails is certainly _way_ more than I'm doing for other people. I'm just saying if you _are_ going to comment on this stuff, it would be nice if you transferred some of your greater understanding with your comments.
- ritchiea 13y agoThanks for responding to these threads. While the "beautiful & unique snowflake" stuff is a bit much, I appreciate developers standing up for simplicity in design. The architecture astronauts try to take the intellectual high ground by providing a complex solution and it drives me nuts. We should be working to make our code less complex not more complex.
- VeejayRampay 13y agoWell I was probably the only one, but I didn't even know you could use valid? with a custom context (which apparently calls the validations on that context). The more you know...
- jonahx 13y agoNon-facetious question: In what sense do you consider your solution simpler than the one suggested by the article? The only additional complexity I see in their solution is creating one more class, but that seems like a surface complexity. I respect your opinion but it seems that you aren't really addressing the advantages the article is claiming their design has. The likelihood of change is much different for the policies than for the basic domain model attributes of a ticket, and this to me is the key reason to prefer the policy design that I'd like to hear you address objectively. Based on your comment about swappable policies, I'd guess that your position is to wait until you need to pull it out before you do. But if you can see a likely change coming and there is no cost to designing for it, why not do it? Is your bone of contention with the idea that we can assign reasonable probabilities to what is likely to change; with the idea that there is no cost to pulling the policy object out now; with both of those things; or with something else entirely?