There's ways to do cleaner SRP that allow moving email delivery out of a model and controller and still be flexible for emailing different things based on type of user, etc.; it's merely a matter of how you solve the problem. simple.rb is the simplest thing that'll do basically the same exact thing that the original author intended - separate object, fast to test, blah blah blah. Introducing DI to override delivery is super simple and can be a lambda/Proc or method; WithDeliveryEmail
doesn't give a shit, as long as it has a call
method accepting the user to deliver the email to.
Is the solution of moving email delivery outside the callback of the user sensible? Can other developers figure out what the side effects of saving a user from the controller immediately? Does creating a user always ensure that an email is delivered? If the system does what it needs to do and all those can be answered with yes, I think it's a reasonable solution.
@joshuaclayton
I think your code does too much just to avoid a conditional. I hate conditionals as much as the next guy, but sometimes they can't be avoided. Nice code though!
Regarding the article, the author ended up creating a
AdminUsersController
, which would have been enough to avoid the conditional, which also revealed the root problem: TheUsersController
was not following SRP (creating two "types" of users).