[danso]

Oh boy, is DHH going to jump into this too?

As an intermediate Rails developer...I don't understand why this if/else forest:

    if user.blacklisted? # They've been banned for bad-behaviour
      fail! “You can't book a Grouper at this time”
    elsif grouper.full?
      fail! “This grouper is full, please pick another date”
    elsif grouper.past?
      fail! “This grouper has already occured!”
    elsif user.has_existing_grouper?(grouper)
      fail! “You’re already going on a Grouper that day”
    elsif ticket.confirmed?
      fail! “You’ve already confirmed this grouper”
    end

-- can't simply be part of the Ticket object? The Ticket clearly has a relation to User and Grouper and talking to those objects (i.e. `if user.not_blacklisted? && user.not_booked(self.date)`) don't violate Demeter...how is making a policy object cleaner than just using a Concern that is included into Ticket? The controller then just needs to ask for `ticket.confirmable?`

edit: OK, not ask for `ticket.confirmable?`, but rather, try to `save` the ticket and the Ticket constructor/validators can pass on the errors to the controller.

[tomblomfield]

I think ultimately it's a philosophical difference; the problem with very many large Rails deployments is that the ActiveRecord models are the core of the application, and DHH's conception of Rails seems to encourage this.

This tends to cause a lot of problems - it's hard to exercise a single model in tests without having a very large number of associated objects present. This also tends to require them to be in the database. This makes your tests very slow, and your application very tightly coupled and hard to refactor.

If you rethink your Rails application and make models responsible for only data validation & persistence, you tend to avoid these kinds of problems. You then need somewhere else for the "core" of your application logic to reside. We suggest these new concepts are Interactors, Policies (and Decorators - more to come in a new blog post).

[phillmv]

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.

[tomblomfield]

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"

[phillmv]

>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?

[mnarayan01]

> 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.

[Jeff_Dickey]

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.

[phillmv]

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.

[karmajunkie]

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.

[briantakita]

This seems like, in essence, a simple function placed in a domain context. For example, in node.js, I would make this a module (a file that exports a function). The module would be named and nested in a directory structure that matches the domain.

It seems that the logical ontology is based on ruby's objects. Unfortunately, this makes the solution much more convoluted than a simple function exported in a module.

[phillmv]

The solution you describe is isomorphic to the one proposed in the blog post, which I think you recognize?

But both leave a lot to be desired. I personally think in both cases they are a code smell, and we ought to either introduce an object whose explicit job it is to worry about these inter domain interactions or pile it up as a mixin into the appropriate domain models.

[briantakita]

The solution I describe is basically a functional approach. The function is responsible for the policy concerns over all data in the system.

The function itself can be decomposed into smaller functions.

If you need extensibility, you could make the function composable and register additional pieces to the policy. Most codebases don't need this sort of extensibility.

Simple constructs & consistent, accurate, precise naming is preferable to complicated architectures.

[phillmv]

:%s/function/module/ and no meaning is lost :).

I've yet to catch up on my lisp and fully grok the "functional paradigm" and, admittedly, passing around consts is less elegant than passing around function pointers, but what you're describing sounds isomorphic to me in terms of code organization. Look at the blog post; he's just passing a one method class, which… is basically a less elegant function.

[briantakita]

> :%s/function/module/ and no meaning is lost :).

Javascript has functions, from the function keyword. Node.js uses commonjs. In commonjs, every file is a module. You can use

module.exports = <value>;

You can access the module by using require.

var moduleValue = require("path/to/module");

It's powerful because it doesn't have the namespace collisions that Ruby has. Everything is not global all the time. modules are also an elegant way of holding private state using closures.

Having a class with a single method is the Ruby way of categorizing this method within the domain.

[mjbellantoni]

I know this isn't the point of your comments, but I'll take the opportunity to point out that Ruby has a case statement that reads a lot better for code like this:

  case
  when user.blacklisted?
    # ... code ...
  when grouper.full?
    # ... code ...
    # ... and so on ...
  end