7 ms·

I rewrote the code in this example to use the "Beginner's Version" of Rails (sigh). You judge which you like better: https://gist.github.com/dhh/9333694 https:/

by dhh 13y ago

I rewrote the code in this example to use the "Beginner's Version" of Rails (sigh). You judge which you like better: https://gist.github.com/dhh/9333694 https://gist.github.com/dhh/9333694

dhh 13y ago
Here's another version that doesn't even use private methods in the controller and uses a PORO for the email grouping: https://gist.github.com/dhh/9333991 https://gist.github.com/dhh/9333991
- danso 13y ago
  Thanks for writing this. While I often look for ways to take external-interaction-responsibility away from Rails objects, the OP didn't really make sense to me, at least in terms of saving time and making things more logical. However, there's a distinction that has to be made in your example and the OP's. In the OP, the failure of the Interactor, including the delivery of emails, would cause the controller to enter the "failure" branch: if interactor.success? redirect_to home_path else flash[:error] = interactor.message render :new end Whereas your example, the controller would take the successful branch if the data model was saved, regardless of whether email delivery failed: if @grouper.save ConfirmedGrouperEmails.new(@grouper).deliver AssignBarForGrouper.enqueue(@grouper.id) redirect_to home_path else render :new end So we're not comparing apples to apples here. However, as a layperson, I'd have to agree with you: Why should the error be raised to the grouper-creating user, when it should be going to the part of the system that handles mailing? But maybe the actual details are more complicated than that...
  - dhh 13y ago
    Why would the delivery of the emails fail? Because your SMTP server is down? That's an exceptional state, handle it with exceptions -- not with conditions. Or maybe because the email addresses are invalid? Handle that when they are captured. It's way too late for that here.
  - Axsuul 13y ago
    The failure of delivery of emails should be handled gracefully such as within a message queue that gets retried until it succeeds or dies.
- iamwil 13y ago
  I can't seem to find it on Google. What's a PORO?
  - cheshire137 13y ago
    Plain Old Ruby Object, as opposed to an ActiveRecord model.
  - deleted 13y ago
    [deleted]
  - dragonwriter 13y ago
    Never seen the term used before, but by analogy to POJO and POCO it would have to be a "Plain Old Ruby Object".
  - deleted 13y ago
    [deleted]
  - the_fury 13y ago
    Plain Old Ruby Object
- rubiquity 13y ago
  I think a lot of blog posts use trivial and contrived examples for the point of explaining the concept. There's not a lot of people out there that are good at explaining complicated refactorings. Katrina Owen comes to mind as one of the few that is good at explaining such refactorings. Perhaps if someone made a post like this with starting code that was more complex it would be a better example, and also harder for you to counter it with a couple gists ;)
- jlangenauer 13y ago
  Isn't ConfirmedGrouperEmails here just an Interactor / Service Object anyway, but one that is derived from ActionMailer? To my mind, this code is much clearer, which proves the OPs point.
tomblomfield 13y ago
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 13y ago
  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.
joenas 13y ago
Say you wanted to do the same thing from a Rake task or the console. Wouldn't an interactor be a nice thing to have then?

by dhh 13y ago

I rewrote the code in this example to use the "Beginner's Version" of Rails (sigh). You judge which you like better: https://gist.github.com/dhh/9333694 https://gist.github.com/dhh/9333694

dhh 13y ago

Here's another version that doesn't even use private methods in the controller and uses a PORO for the email grouping: https://gist.github.com/dhh/9333991 https://gist.github.com/dhh/9333991

danso 13y ago
Thanks for writing this. While I often look for ways to take external-interaction-responsibility away from Rails objects, the OP didn't really make sense to me, at least in terms of saving time and making things more logical. However, there's a distinction that has to be made in your example and the OP's. In the OP, the failure of the Interactor, including the delivery of emails, would cause the controller to enter the "failure" branch: if interactor.success? redirect_to home_path else flash[:error] = interactor.message render :new end Whereas your example, the controller would take the successful branch if the data model was saved, regardless of whether email delivery failed: if @grouper.save ConfirmedGrouperEmails.new(@grouper).deliver AssignBarForGrouper.enqueue(@grouper.id) redirect_to home_path else render :new end So we're not comparing apples to apples here. However, as a layperson, I'd have to agree with you: Why should the error be raised to the grouper-creating user, when it should be going to the part of the system that handles mailing? But maybe the actual details are more complicated than that...
- dhh 13y ago
  Why would the delivery of the emails fail? Because your SMTP server is down? That's an exceptional state, handle it with exceptions -- not with conditions. Or maybe because the email addresses are invalid? Handle that when they are captured. It's way too late for that here.
- Axsuul 13y ago
  The failure of delivery of emails should be handled gracefully such as within a message queue that gets retried until it succeeds or dies.
iamwil 13y ago
I can't seem to find it on Google. What's a PORO?
- cheshire137 13y ago
  Plain Old Ruby Object, as opposed to an ActiveRecord model.
- deleted 13y ago
  [deleted]
- dragonwriter 13y ago
  Never seen the term used before, but by analogy to POJO and POCO it would have to be a "Plain Old Ruby Object".
- deleted 13y ago
  [deleted]
- the_fury 13y ago
  Plain Old Ruby Object
rubiquity 13y ago
I think a lot of blog posts use trivial and contrived examples for the point of explaining the concept. There's not a lot of people out there that are good at explaining complicated refactorings. Katrina Owen comes to mind as one of the few that is good at explaining such refactorings. Perhaps if someone made a post like this with starting code that was more complex it would be a better example, and also harder for you to counter it with a couple gists ;)
jlangenauer 13y ago
Isn't ConfirmedGrouperEmails here just an Interactor / Service Object anyway, but one that is derived from ActionMailer? To my mind, this code is much clearer, which proves the OPs point.

tomblomfield 13y ago

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 13y ago
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.

joenas 13y ago

Say you wanted to do the same thing from a Rake task or the console. Wouldn't an interactor be a nice thing to have then?