RE: Should controllers be "smart"?

Your register method is a business logic and shouldn’t be in the model.
The model is usually only for storing and retrieving data.

The simple fact that you want an email to go out after someone registers
is further implication that the register method is business logic.

Chris

I know that the register method is business logic, which is why I put
it in the model. Isn’t that where the business logic is supposed to
go? Where else would you put it?

Pat

On 5/4/06, Tobias Lütke [email protected] wrote:

just delegates to the UserMailer. This keeps the logic in the model
Rails mailing list
[email protected]
http://lists.rubyonrails.org/mailman/listinfo/rails

Keeping with the welcome email example, would you then just make your
model a bit more fine-grained?

controller code

u = User.register(…)
u.deliver_welcome_email

I don’t know, I’d still stick the email delivery call inside
User.register. All it does is create the user and deliver the email.
If you don’t want to send the email, just call User.create instead.

Pat

If you don’t want to send the email, just call User.create instead.
It depends on your requirements, if you have some situations where
you may want to create them without delivering, I’d do it like tobi
has there.

However, I can say that we currently send our welcome emails from our
equivalent of ‘User.register’


Cheers

Koz

+1 to what Tobias said.

The Rails approach to MVC is to push a lot of things down into the
model.

Whether or not you think this is the right way to do things (which,
for the record, I don’t), you shouldn’t try to fight it when
developing Rails code.

Here’s an example from some code I’ve been working on lately to
illustrate the point:

I hate dropdowns, so I don’t want to use the standard date picker for
a date field on my model. I’d rather have a text field and parse the
date, but this obviously requires validation. So, here’s the code
from my model:

(Just take date_is_valid? and parse_date as givens for now.)

attr_writer :date_as_text

def date_as_text
@date_as_text ||= (date && date.strftime(‘%d %b %Y’)
end

validate :validate_date_as_text
def validate_date_as_text
if @date_as_text
if date_is_valid?(@date_as_text)
self.date = parse_date(@date_as_text)
else
errors.add(‘date’, must be a valid date’)
end
end
end

“But wait!”, I hear you cry, “Why are you doing this in your model?
It’s clearly a user interface issue, and belongs elsewhere!”

In theory, you’re absolutely right. In practice, Rails puts all
validations in the model and you’re best not to fight it.

Here’s what happens if I try to handle the date_as_text field in the
controller:

def new
@my_model = MyModel.new
if request.post?
date_is_valid = date_is_valid?(params[:date_as_text])
@my_model.date = parse_date(params[:date_as_text]) if
date_is_valid
if @my_model.valid? and date_is_valid
@my_model.save
else
@my_model.errors.add(‘date’, ‘must be a valid date’) unless
date_is_valid
end
end
end

I feel like I’m fighting Rails to pull this off. I’ve got to run the
model validations manually. I’ve got to push errors from the
controller back into the model so that they can be passed to the
view, and I’ve got to be careful to do it after I do the save. I
can’t really have model validations for the date field too, because
then the user might see multiple, conflicting errors for the one
field. Editing a record requires more code, because I have to grab
the date and convert it to text before displaying the form.

So, as much as I believe that code like this should live in the view/
controller, it’s not worth fighting the Rails way of doing things to
make my ideological point. :slight_smile:

Pete Y.
http://9cays.com

On 05/05/2006, at 11:05 AM, Tobias Lütke wrote:

Since I accept I am wrong about my (previous) MVC definition, I am
wondering how you do specific business logic things in the Rails model
like:

If you have a register method, do you return the model AND true/false so
it can be inserted back into the view if validation fails and the
controller knows what view to show next?

Do you pass a hash to register and call self.save inside the method?

What if you are updating multiple tables and want to perform it in a
transaction (common business logic). Does your register method find the
other models and update/save them all in a transaction within the
register method?

Is it bad practice to call model.save in controllers, since they seem to
be only to transfer user input/output data to/from views and models?

Tobias - Is typo a good source to look at as an example of how to
correctly use MVC in rails?

Chris

Pete Y. wrote:

+1 to what Tobias said.

The Rails approach to MVC is to push a lot of things down into the
model.

Whether or not you think this is the right way to do things (which,
for the record, I don’t), you shouldn’t try to fight it when
developing Rails code.

Here’s an example from some code I’ve been working on lately to
illustrate the point:

I hate dropdowns, so I don’t want to use the standard date picker for
a date field on my model. I’d rather have a text field and parse the
date, but this obviously requires validation. So, here’s the code
from my model:

(Just take date_is_valid? and parse_date as givens for now.)

attr_writer :date_as_text

def date_as_text
@date_as_text ||= (date && date.strftime(‘%d %b %Y’)
end

validate :validate_date_as_text
def validate_date_as_text
if @date_as_text
if date_is_valid?(@date_as_text)
self.date = parse_date(@date_as_text)
else
errors.add(‘date’, must be a valid date’)
end
end
end

“But wait!”, I hear you cry, “Why are you doing this in your model?
It’s clearly a user interface issue, and belongs elsewhere!”

In theory, you’re absolutely right. In practice, Rails puts all
validations in the model and you’re best not to fight it.

Here’s what happens if I try to handle the date_as_text field in the
controller:

def new
@my_model = MyModel.new
if request.post?
date_is_valid = date_is_valid?(params[:date_as_text])
@my_model.date = parse_date(params[:date_as_text]) if
date_is_valid
if @my_model.valid? and date_is_valid
@my_model.save
else
@my_model.errors.add(‘date’, ‘must be a valid date’) unless
date_is_valid
end
end
end

I feel like I’m fighting Rails to pull this off. I’ve got to run the
model validations manually. I’ve got to push errors from the
controller back into the model so that they can be passed to the
view, and I’ve got to be careful to do it after I do the save. I
can’t really have model validations for the date field too, because
then the user might see multiple, conflicting errors for the one
field. Editing a record requires more code, because I have to grab
the date and convert it to text before displaying the form.

So, as much as I believe that code like this should live in the view/
controller, it’s not worth fighting the Rails way of doing things to
make my ideological point. :slight_smile:

Pete Y.
http://9cays.com

On 05/05/2006, at 11:05 AM, Tobias L�tke wrote:

Tobias - Is typo a good source to look at as an example of how to
correctly use MVC in rails?

I’d be interested in hearing the “correct” way to do things too. I’ve
read the Agile… book but it still left me with some architectural
“greyness” about RoR, which I why I balked and did my last project in
straight-up, procedural PHP.

ducks

So, I was looking through the typo code and have a question. It looks
(and I might be way off) that the models (I looked mostly at blog.rb)
seem pretty tightly coupled to the overall web appication. Is this
correct? I was under the impression that your models should be as
independent of a specific application as possible or am I way off base
here? I thought your models should enforce business rules, like can’t
take out more money than you have in your account, etc. I was thinking a
good test would be if you can take your existing models and use it to
build say a stand-alone application with little or no modification–that
means your models are well designed. Is this the completely wrong
assumption of what the M is in MVC??

Thanks,

Chris

gravy face wrote:

Tobias - Is typo a good source to look at as an example of how to
correctly use MVC in rails?

I’d be interested in hearing the “correct” way to do things too. I’ve
read the Agile… book but it still left me with some architectural
“greyness” about RoR, which I why I balked and did my last project in
straight-up, procedural PHP.

ducks

On 5/5/06, Chris B. [email protected] wrote:

means your models are well designed. Is this the completely wrong
assumption of what the M is in MVC??

You could have a mix of models that are application specific and some
that
are not. But your application logic belongs in the model 1) because
it’s
easier to write it that way 2) because then you can reuse that logic in
different controller/view combinations. In theory you should be able to
build a new user interface that reuses your models to implement the same
application logic. The controller handles the conversation between the
views and the models and handles the “control flow” logic i.e. what
screen
to show next after we’ve done with this one.

That’s how I make sense of it all.

cheers,
Ben

I think that type of DATA validation should go in your model as you have
it.
Validating a date format has nothing to do with your interface (view)
and
nothing to do with the application logic (controller).

I think there is a difference between what people call “business logic”
and
“application logic”.

Business logic is the stuff you need regardless of the framework you are
using. This includes your data model and data validation RULES. It’s
all
the rules you would follow even if your business ran on paper and filing
cabinets.

Application logic is the stuff that may change depending on the
framework
you are using. This is your controller and it glues the backend to the
front-end, deciding which views to show and which models to send
messages
too.

Uhhh this is absolutly positively wrong.

On 5/4/06, Chris B. [email protected] wrote:

Your register method is a business logic and shouldn’t be in the model.
The model is usually only for storing and retrieving data.

The simple fact that you want an email to go out after someone registers
is further implication that the register method is business logic.

Business logic belongs into the model unless its http related.

In the case of sending an email after a signup i think the controller
is the correct please to put this code. In a sense writing a email as
part of a signup procedure is directly tied to http, you wouldn’t send
the user a email when you create him on the console for instance.

However i would put the call to send the email in the model. My user
models usually have a method like user.deliver_welcome_email which
just delegates to the UserMailer. This keeps the logic in the model
but makes it explicit.


Tobi
http://shopify.com - modern e-commerce software
http://typo.leetsoft.com - Open source weblog engine
http://blog.leetsoft.com - Technical weblog