Hacker News new | ask | show | jobs
by phillmv 4468 days ago
It sounds like you're letting "ease of test maintenance" drive the architecture of the rest of your app; I'm inclined to accept that incentivizing test creation will lead to more confidence in the face of changes but am somewhat unconvinced it's 1:1 with "less coupled, more maintainable" a priori.

Case in point, I'm really uncomfortable with class names with verbs. Typing CanConfirmTicketPolicy.new(ticket: ticket).allowed? smells bad to my fingers.

It strikes me that you haven't eliminated the coupling? because the CanConfirmTicketPolicy still depends on different domain objects. Kind of by definition, you can't remove it because it's explicitly operating on User, Group and Ticket; the main difference to me seems that they're easier to mock?

I would argue that this ought to be either an explicit inter domain class that models the interaction between Users, Group and Tickets - some appropriate noun like Purchase or GroupActivity or whatever - that can then be solely responsible and reused as appropriate.

You have a bit of text that explains your decisions,

>We could move the logic to a controller mixin, or define the method on the ApplicationController, but it would still not be available in our view

Could you not just define a helper method, then? Helpers are available in both views and controllers if memory serves, are mixed in by default, are equally easy to test as your verbed policy object and have the (minor) added benefit of not polluting your namespace while being equally easy to reason about - it's unclear to me how can_confirm_ticket?(ticket) would necessarily be inferior.

If you're really interested in putting these in models (which is also acceptable) then a regular concern namespaced to your preferred model would work just as well.

Am I missing something? I would like to understand your use case but I don't seem to get it.

2 comments

I have no problem with verbs in class (or module) names - I'd actually say we should encapsulate more of this "activity" or "verb" behaviour in single-responsibility objects, and not within ActiveRecord models.

Eg, a ConfirmGrouper.perform object (http://eng.joingrouper.com/blog/2014/03/03/rails-the-missing...)

Uncle Bob's talk gives good reasons for this pattern: http://www.confreaks.com/videos/759-rubymidwest2011-keynote-...

Basically, your data-persistence objects (ActiveRecord models) become more composable because they do fewer things and rely less on associated objects being present to work properly.

Your "interactor" objects which perform "verb"-style activities can also be broken down into very small units with single-responsibility and then composed with each other to perform more complex interactions.

Putting all of this logic into models makes it very very hard to break apart units of behaviour and re-compose them.

This is what I mean by "less coupled, more maintainable"

>Basically, your data-persistence objects (ActiveRecord models) become more composable because they do fewer things and rely less on associated objects being present to work properly.

I agree with this entirely, and elsewhere in this thread have said so, and I'm not advocating fat models.

I'm more questioning the precise way in which you've chosen to perform your refactoring. To me, that-method-in-partcular seems like a code smell, and I've not been convinced by your blog post compared to the alternate methods I've suggested in my comment. Does that make sense?

> It sounds like you're letting "ease of test maintenance" drive the architecture of the rest of your app; I'm inclined to accept that incentivizing test creation will lead to more confidence in the face of changes but am somewhat unconvinced it's 1:1 with "less coupled, more maintainable" a priori.

I've thought of making a similar comment on _many_ other articles but never have. This is actually the first time I've ever even _seen_ anyone else saying it. I wonder if we're relatively unique or if many other people feel the same way and simply (like myself) don't actually comment on it.

Difficulty of testing is a generally reliable indicator of a design that's in deep pain. [Growing Object-Oriented Software, Guided by Tests](http://www.growing-object-oriented-software.com) is easily one of the two or three most influential technical books I've read in my career — and the book's only five years old.

When I'm spelunking through some dank and dimly-lit Rails-model God Objects, I often fantasise about how things would be different if certain influential developers had read that book or one like it before setting out, and had taken the lessons to heart. Then I swap the batteries in my headlamp, and continue my descent.

Not to dull the shine of my own brilliance but I'm pretty sure I've seen dhh say something along those lines in his previous commentary.

I've spent a lot of time thinking about this subject, and have only recently begun to feel confident enough to speak with some authority on the subject.

I think people aren't thinking critically about their code in quite the right way, despite their blog posts on the topic, but I've hitherto been unable to compose my thoughts outside of comment boxes.

I think you might be missing the point of tests. Its not "ease of test maintenance" that's driving architecture; its testing that elucidates bad design. The fundamental problem in the rails community is that instead of evolving architecture, we've added tooling to decrease the pain (e.g. guard/spork, zeus/spring, et al) rather than fix the way we write code that makes testing easy (easier?) and (more) painless.