Hacker News new | comments | show | ask | jobs | submit login

Maybe this is naive, but what's wrong with just putting `UserMailer.welcome_email(user).deliver` in the controller, right after the user is created? In my mind, delegating an email to be sent should be a job for the controller.

IIRC, the Rails ideology says that "Models should not know about any part of the application except for their own datastore." Even though the :after_create hook is the "Rails way", doesn't this violate their own practices?

It's okay if that's the only case where the email should be sent, and other means of user creations should never, ever do that. It might be an issue if, for example, the email should be sent always, no matter how the user was created.

Edit: basically, the question is "which process involves sending the email". It might be "creating a user," or it might be "using the 'create user' form". At some point you might need a more abstract solution, of course — see things like Listeners, DHH's Concerns, AOP or whatever you like. But the thing is, if you don't delay too long, you can retrofit your codebase with it, so don't worry too much.

Unless you're coding up a nuclear plant or storm barrier or the moon-carving laser. Please don't use webappy principles to code drivers for the moon-carving laser.

We had a case at work for not doing this. We had a section of our app where users could set up their profile, for example /user/profile (scoped to them by their session). Imagine it is more than just CRUD, and has a few bits of complicated logic (in reality there are multiple controllers - it's a big profile :D).

We then wanted to add the ability for admins to edit profiles, so we created /admin/users/[id]/profile. If the logic wasn't in the model we would have to repeat it in both controllers (the views can easily be reused by having the form elements in partials).

Of course — complicated logic is a good hint that code maybe should be moved somewhere else, be it the model or some manager class. Just like you moved (or maybe had from the start) the form elements into partials. It's probably not something to obsess about too early.

The thing that bugs me the most is how both the article and some responses here go into heavy absolutes. Different domains have different needs, and what is "readable" changes between them, not just by size, but by shape as well. And yes, shape and size might change as the app lives (my current codebase being a great example), but you can't really predict everything.

This is pretty much exactly what DHH advocates for, and for a simple case like this (creating a user, sending an email) it works great.

At some point though, your controllers are going to grow. Multiple code paths depending on the data submitted, depending on the current state of the application, etc. At this point, building service classes such as CreatesContact can become very useful, not to mention much easier to test and reuse.

Evan Light's article at http://evan.tiggerpalace.com/articles/2012/11/21/use-rails-u... advocates "Use Rails until it hurts" which I think is a very good mantra to keep in mind when working with Rails.

It really depends on what the system needs and how you are reusing functionality. Creating a user on the front end of the site with a new user sign up makes sense to send an email, but if you have a back-end admin area, creating a new user there probably wouldn't be a good idea to send out an email in many cases.

In short, use your head. There is nothing wrong with breaking out this functionality into two actions.

I like it too, and have this line of code in a few apps. It's not technically right because, in MVC, your business logic belongs in your models.

Imagine a situation where you want to bulk-create Users via script (or, simply, not via your controller "create" action). Shouldn't those bulk-created users get emails too? It depends on your business logic, hence, your model.

Well, the business logic that the given model is responsible for belongs in the model. For instance, calculating an order total belongs in the Order model if you are building a shopping cart. However, you shouldn't break separation of concerns just to keep 100% of the business logic inside your object models.

Having tightly coupled objects that depend on other classes in your application that you wouldn't expect is worse than having a little bit of business logic in your controller (or some service object), in my opinion.

Guidelines | FAQ | Support | API | Security | Lists | Bookmarklet | DMCA | Apply to YC | Contact