[tomblomfield]

I've spent the last few months re-writing a medium-sized code-base (several hundred thousand LoC) that looks like your version into code that looks like the blog version. Test suite run time has dramatically decreased, code is more loosely coupled and we see far fewer bugs.

> You obviously need to choose the patterns that fit the problem you’re trying to solve – it’s rarely one-size-fits-all, and some of these principles may be overkill in a very simple 15-minute blog application. Our objection is that Rails feels like it’s a Beginners’ Version, often suggesting that one size does fit all

This quote from the post (minus the "Beginners" jibe - sorry!) is my core objection. Your example works fine if you're working with a simple application. But the "Rails Way" feels like it starts to fall over when you get to medium or large codebases.

I feel like Rails could benefit hugely from a few core "Advanced" features that help you grow a small application into a larger, functioning business. Sure, you can add them yourself, but then you lose the advantage of shared standards and strong architectural conventions. Every new developer we hire has to learn what exactly we mean by "Interactors" or "Decorators" or "Policies".

[dhh]

Yes, rewriting a system after it's already settled and designed can indeed bring a cleaner code base about. But don't confuse that with "better architecture".

Don't even get me started on the hand-wavy "loosely coupled".

If this is a poor example, pick a good example. I'll be happy to code ping pong you whatever example you choose.

One way to spend hundreds of thousands of LOC on an application is to stuff it with needless abstractions. That doesn't make it "advanced", and it's not Rails that's falling over, it's probably just some shitty code.

Maybe that's a clue that your chosen paradigms weren't that helpful. Or rather, that they convoluted the code base instead of clarified it. It's certainly a red herring that you need to teach your abstractions to new developers and that it's an endeavor to do so.

POROs are great, though. Our app/models is full of them. We even added app/services too. The trouble I have is with people who, like you, fall in love with the flawed notion that their application is such a special snowflake that it deserves "advanced techniques". Bullshit. There are good techniques and bad techniques. Size of the application is rarely a factor, except in ballooning the ego of the programmer.

Anyway, the invitation stands. Present any piece of real code and we can ping pong on it. I enjoy talking about specifics and improving real code. I detest "oh that was just a poor example, but the general principle is..." kind of debates. If you can't produce a good example, you don't have a general principle.

[nthj]

> POROs are great, though. Our app/models is full of them. We even added app/services too. The trouble I have is with people who, like you, fall in love with the flawed notion that their application is such a special snowflake that it deserves "advanced techniques".

The main difference I see between the Rails philosophy and most of these "Use SRP and Interactors and things!" blog posts are that Rails is way more interested in using the correct tool for the job, and most of these blog posts are of the "use this for everything and every one!" variety.

I have one project right now that I decided I wanted a Command for one action. I'm accepting input from a webhook that I can perform asynchronously, it interacts with like 5 different models, and I wanted to beat up on the tests pretty thoroughly. So I did that.

Elsewhere I'm using concerns to make like 4 different models easily sortable.

And other places I'm using Module#concerning to keep models organized.

ONE TRUE WAY is overrated. I'd rather use whatever works given the context.

[gregmolnar]

"ONE TRUE WAY is overrated. I'd rather use whatever works given the context." That's it.

[andycroll]

This. This. This.

[AndyDavis3416]

I hope I am not sticking my head into the lion's mouth here, but here (https://gist.github.com/BiggerNoise/9334673) is a link to a gist of some code in one of our apps.

Cases have Tasks and Activities. Activities are always associated with a case, and may be associated with a task.

In the controller, when assigning a case, one would type: AssignCaseCommand.new(params).execute!

The command class implements the Command pattern (the pretty well established name for interactors) and is a Composite as well.

BTW - the same command that assigns the tasks of a case is also used to merely assign a task. And, if we're just recording an activity, we would just use the create activity command.

I think it would be insanity to try and capture the rules associated with re-assigning tasks in the controller. (I am pretty sure that you would agree.)

--Minor Edit for clarity--

[tinco]

I think it's great that you stick your head into the lion's mouth, even if he chewed it clean off it's a great contribution :)

I think the main problem with your command pattern approach is that you didn't implement the full command pattern. At least, I assume you didn't, you didn't actually show the controller. The command pattern has one extra class, the invoker. The invoker takes commands, and invokes them. A great example of this is the worker queue, it takes generic worker objects that have an 'execute' method.

It is never the class that creates the worker objects, that executes them. Yet that seems to be the way you're going to implement the controllers that use these commands.

What I think you've built instead is an adapter pattern. You have abstracted a class of problems in a way that they have a common interface, so that in your controllers you have a uniform way of invoking them.

This is where I think dhh sees the muddiness in your code. Why are you abstracting your interaction from the controller, which is supposed to be the exact thing that should be controlling the interaction.

The only reason for that would be that the same interaction is happening in different controllers in exactly the same way. And that's something that dhh claims does not happen in real applications, or at least he demands an example.

[AndyDavis3416]

Honestly, I was disappointed in his response. With all his harrumphing about real examples, I would have expected him to make an effort to understand a real example. But, as I said elsewhere, the downside of real examples is that they require quite a bit more effort to grasp than contrived ones.

To address your comments:

While it is true that one of the great features (and frequent motivations) for using the Command pattern is the separation of the creation and invocation, it is not the only motivation. The two big payoffs that we are getting are:

- Separation of invocation from the knowledge of how it is carried out

- Composition of complex commands from smaller, simpler commands

There's no requirement that the invoker is never the same as the creator. It's just that most often it isn't, particularly (as you noted) in a worker queue system. That's really just a feature of the implementation that the command readily enables rather than a fundamental property of the pattern. For us, sometimes it is, sometimes it isn't; that's not the primary payoff that we're going for.

I can't support the characterization of this as an Adapter. This is fundamentally behavioral. I have a class that represents an action that I want to have happen. All of the places that want this action do not need to know what is involved, they only need to create the command and hand it off/invoke it.

FWIW - I have five different places in my software that create an AssignCaseCommand. - One controller

- Three commands that can create this as a sub command

- A utility job

Plus that simple concept is available to any custom implementation code that we write.

[tel]

The Command pattern always reminds me of free monads. By building a new free monad you can encode the structure and logic of a computation statically and then evaluate it repeatedly. One of the things that I think is missing with the command pattern is that it's probably not as easy to decide to, for instance, execute your commands in a pure environment.

This is, of course, a primary advantage of having a static representation of the domain-level demands of your code—you can choose many ways and many environments to execute it within. It's a function I use in real code all the time.

[dhh]

I asked for specific code, you gave me specific code, so here's how I would have structured that without the (imo, needless) command pattern: https://gist.github.com/dhh/9348053

It includes a lot of more context than your example, so I was guessing liberally at the missing pieces. But hopefully this is still easily digestible.

[dhh]

If reassigning a case is part of the domain, it belongs in the model.

I don't know how to put this gently, but this concoction looks like a hot mess with no separation between modeling and controlling.

These commands have a ton of domain logic all tangled up with activity reporting, formatting, and parameter parsing. It looks like a big ball of mud to me :(

[gnaritas]

> It looks like a big ball of mud to me :(

That is of course the most popular of all architectural patterns. :)

[akmiller]

Most popular by what metric?

[gnaritas]

Practiced.

[AndyDavis3416]

Perhaps that's why the OP used a simple example. Complex examples are pretty hard to get your head around at first glance; even sane systems can be perceived as a 'hot mess' upon a quick read without any context.

I disagree that re-assigning a case belongs in the case model. It is a complex interaction between several models. Extracting it to a command object allows all of the rules to be easily understood.

[halostatue]

It's a model, all right—but it may not be part of the “Case” model. It might be part of a CaseAssignment model that is (for lack of a better word) a virtual model. Or it's part of the controller that does the changes. It mostly depends on how your code gets used and where it gets used from.

When I did something similar[1], I implemented it as a “virtual” model and applied validations to make sure that the PlanChange was properly constructed, and then performed the change in a transaction. I was able to test the hell out of it both with and without actual database interaction. But I was able to use it in three different locations and removed lots of special case code that had built up over two years of development.

You probably don't need Commands. You probably need something to orchestrate manipulating multiple ActiveRecord objects simultaneously in a transaction. It's still a model, but it may not be the model that you thought it was.

[1] Account from Plan [a] to Plan [b] that has a different quota, affecting Device assignments, possibly including moving some devices to Plan [b], some devices to Plan [z] (a default plan), and disabling some devices entirely because there's no quota available anymore.

[AndyDavis3416]

Although I would never call that a model, I think that we're doing similar things for similar reasons.

Command, Virtual Model, Verb Model, Interaction. If I were working on your team I am sure we could agree that one of those would do for a name.

[hkarthik]

I feel like showing off some tests would help when the example is this complex. That would give a bit more context in terms of what you're trying to accomplish here.

[AndyDavis3416]

True, the tests do demonstrate what can be accomplished, but that kind of gets back to my point: Any example complex enough to unambiguously warrant the use of a command pattern is going to involve non-trivial effort to learn the context. Asking that effort of someone reading a blog post is a bit much.

Unfortunately, it's easy to dismiss simple examples as not being worth the complexity and demand real examples which can be dismissed because they cannot be easily understood.

[tomblomfield]

My contention is that there often are times when the complexity of the business requires a fairly extensive set of operations to occur in concert, including the creation of several models, email-sending etc.

In this case, the Right Place (tm) to put these is in POROs / Interactors that can be tested in isolation and re-used across the code-base if necessary. This code should not live in controllers and certainly not in ActiveRecord models.

It sounds like you agree with that, since you're using them in your own codebase?

In that case, is there not an advantage to standardising the API for these POROs in Rails? Simple methods like fail!, success?, failure? and message

Another way of asking the question - why do so many Rails developers fail to realise that they can use these concepts? Why are so many Rails codebases packed full of enormous ActiveRecord models that are crippled with numerous before_save callbacks?

Sure, shitty coders write shitty code. But can't you lend them a hand?

[cgag]

I don't get why POROs would need some special API. I also don't get why you're pulling code out into things like ConfirmGrouper that still depend on the DB. Why not a true PORO and something like ConfirmGrouper.for(grouper)? Why not just write methods that return bools?

Though I don't know if I agree that the actual sending of emails belongs in a PORO either and not a controller. It seems like determining whether or not emails need to be sent is the kind of logic I would put in a PORO and do the actual email sending from a controller method as in DHH's example.

Edit: Just saw DHH's second example, that's essentially how I would write it.

[ajsharp]

> In this case, the Right Place (tm) to put these is in POROs / Interactors that can be tested in isolation and re-used across the code-base if necessary.

Key phrase: "if necessary".

It sounds like you're advocating for building abstractions before you actually have a reason to use them. Your reason seems to be "we may need to reuse them in the future" or, "one can see where we might move in a direction where we'd want want to re-use this code", or worse yet, "we're definitely going to build some functionality after this release when we'll want to re-use this stuff."

I've been there, really, I have. Build the abstraction when you actually need it, not when you think you're going to need it, or pretty close to needing it, etc. Because 98% of the time, YAGNI.

> In that case, is there not an advantage to standardising the API for these POROs in Rails?

Not a big enough that I've come across to warrant building domain-specific abstractions and special rules that, as @dhh mentioned, will serve only to raise the barrier to entry to your codebase.

[tomblomfield]

> It sounds like you're advocating for building abstractions before you actually have a reason to use them.

If it sounds that way, I've over-simplified the examples in the blog post in the interests of clarity. I'll take more care in future! The specific interactor in question is used in 4 separate places.

From the post:

> You obviously need to choose the patterns that fit the problem you’re trying to solve – it’s rarely one-size-fits-all, and some of these principles may be overkill in a very simple 15-minute blog application.

[al2o3cr]

"Sure, shitty coders write shitty code. But can't you lend them a hand?"

It's not clear to me that adding more APIs and conventions that said shitty devs won't read about, understand, or even be aware of is actually useful.

If shitty devs are your problem, don't fix the dev tools - FIRE the tools pretending to be devs...

[dablweb]

There are a lot of brilliant developers giving talks at every RubyConf advocating these very techniques...

[djur]

The standard API for POROs is Object (and Class, etc.). Standardizing a new API necessarily makes them no longer "plain".

[halostatue]

The current app I work on has a lot of warts, but only five `before_save` callbacks (there's maybe five times that many total callbacks, although there are lots of validations).

If you really want to standardize the interface, use the tools that already exist: ActiveModel and validations. Your “Interactors” are models; they just happen to be coordinating models that affect multiple ActiveRecord models simultaneously. (I did this for an account plan change system that affected billing and device assignment to a plan. It could easily affect a dozen or more records, all in a single transaction, but was itself easily testable.)

[Killswitch]

> "I detest "oh that was just a poor example, but the general principle is..." kind of debates. If you can't produce a good example, you don't have a general principle." - DHH, March 3rd 2014

That's going in my quote book. Thank you DHH.

[dablweb]

I think that labelling the use of POROs as an "advanced technique" is simply wrong. How could it be? In fact it's simpler than implementing an AR callback (with more complexity behind the scenes than anything!)

You are using app/services and POROs in your own app? So are we. But I guarantee they don't look like yours because there is no endorsed convention.

Why not? Because Rails is still clinging to such an outdated idea as MVC? Pure MVC at a large scale is bullshit.

"Size of the application is rarely a factor". I don't agree. Size of the application is always a factor. A big factor.

What I'd like to see is the community embrace conventions outside the fabled "MVC" paradise. Rails is "convention over configuration", and is content to hold your hand for your first thousand lines of code... but then what?

[jlebrech]

sometimes more abstraction makes for simpler code, it's just that there's now simple presentation code for the frontend and also simple code for persistence.

avoiding a new layer of abstraction because it can just about be done in vanilla rails isn't a solution, it's just throwing more lines of code at the problem.

recently i've found that everything i do at the frontend intermixed with the controller, models and helpers could be made simpler with two way databinding.

it's also simpler, if someone want's to see if it looks right you can plug in some fixtures and tweaking css can be done independantly instead of needing to write something in ruby to affect how something in html looks.

maybe for a project with a large codebase is to use lighter frameworks such as backbone rather than having to do full rewrites in emberjs or angularjs. and let the code rewrite itself, ie. just clean up the portions of code that were only there for presentation.

[nottombrown]

You can't possibly think that there's never a use case for the Interactor pattern. Please watch Uncle Bob's talk: http://www.confreaks.com/videos/759-rubymidwest2011-keynote-...

[dhh]

Let's just say I have philosophical differences with the views presented in that presentation.

Also, I never said never. I said this particular example was a poor example and the general principle derived from it was equally poor. Garbage in, garbage out.

[rhizome]

I can imagine people thinking that this is a strength, but my frustration with that talk is that it isn't presented with much to do with Rails.

[halostatue]

I can possibly think that there's never a reason to reach for a pattern when it's better to let a pattern emerge from your code and then refine it. As far as the video is concerned I personally won't bother to watch it—Bob Martin sells snake oil as “clean code”.

[jfaucett]

I agree 100 percent with the statements in the first two paragraphs on "Part 1 Inflectors". I have first hand experience with every statement you made from testing to managing the callback dependency mayhem. This post seems interesting to me mainly for the test suite benefits alone, being able to maintain independency in the test-suite base would definately save lots of bloated stubbing and/or db hits. Im wondering what kind of time improvements the suite got?