
On logic in a Rails app - alisnic
http://alisnic.github.com/posts/rails-logic/
======
michaelfeathers
The funny thing about the argument that people have about where logic should
go in an application (models, controllers, or separate objects) is that
everyone assumes that there is one right answer for an application for all
time.

We should probably move toward guidance like: "this is when you move your
logic here" and "when this happens, it's time to move it out of the
controller."

For what it's worth, I think it is okay to start doing things the naive way in
Rails as long as you know when to refactor toward a different scheme. The bad
thing is that it is usually earlier than you think.

~~~
joevandyk
Do people have these sorts of arguments in languages like clojure or haskell?

~~~
michaelfeathers
Domain matters more than language for this sort of thing. People tend to have
these arguments around business applications.

------
tinco
I think many people react negatively to this article because they gloss over
the idea that this is a refactoring.

When coding you always make the choice between abstraction and readability. If
you look at the patterns from GoF, the downside of almost every pattern is
that it reduces readability for the simple cases.

Thus, when you write your controllers and your models in my opinion you should
start out doing it 'the rails way' which is having your business logic in your
models, and the controlling logic in your controllers.

Then when your app is starting to feel clunky, your controllers are gaining
weight and your models have too many responsibilities, that is the point where
you start refactoring.

And that is also where you decide wether the logic belongs into a concern, or
a policy, or whatever fancy pattern is hip right now and fits there. (But you
really should make an effort to make clear where logic ends up for someone new
to your code, so be consistent!)

I think rapid development is one of the key features that made Rails big,
don't let these concerns that big apps have drag you down in the beginning.

~~~
jeremyjh
People will spew all these separate classes from the very beginning in the
name of "test-ability".

~~~
alisnic
I don't understand what is the problem of having a lot of small classes? They
are there when you need them, and they get out of the way you don't, and I
think that's the most important thing.

~~~
fbuilesv
Indirection. Every time you add a new class to do something and call it from
somewhere else you've added an extra level of indirection. This can be good
when dealing with complex logic but for many cases it just hinders code
readability.

~~~
alisnic
I am not sure I understand your point, can you elaborate?

~~~
Firehed
When the actual logic resides deep in a call stack, it's more difficult to
debug code, much less bring on new developers. Each additional abstraction
layer is, unsurprisingly, another layer that you're going to have to trace
through to get at the root of the issue.

Similarly, when your logic doesn't reside at the root level, it's easy to
accidentally bypass. Perhaps this is intentional, but I'd argue that
intentionally bypassing certain logic under certain conditions indicates your
logic is poorly factored. My most common example to this is an admin panel:
admins can, as an example, change any user's name, not just their own.
However, this only holds true when in the admin panel. So in your user
controller, you have some sort of permission check ensuring that the user
being operated on is the one authenticated[1], but intentionally don't place
that check in your admin controller. One day in the name of a better UX you
add an AJAX component to this, and forget to copy across that check into the
ajax endpoint. Suddenly I can change anyone's username, despite not being an
admin.

To solve this, we've created a couple of libraries: PermissionCheck and
AuthContext; the former (typically) depends on the latter. AuthContext answers
the questions of who are you (authentication) and where are you (context), and
the permission check wraps up the authorization component: in the (web
frontend | api | admin panel | cronjob | upgrade script) does the currently
authenticated user have permission to do X on object Y? By putting these
checks in the models, it's easy to ensure they're always enforced (a user's
updateName method may call checkUpdateName()->enforce() ); if you want to
side-step (or ramp up) authorization under a certain context, you edit the
model's permission check for that action, rather than one or more controllers.

Could you put that into a different helper library? Probably, if you were so
inclined. However, that leaves you with at least two files to check when you
want to examine behavior, and an inability to use any protected or private
methods from these permission checks.

Having an object that does a lot of stuff is not necessarily the much-feared
"God object", although it certainly can become that. I've learned to become
more fearful of the "God methods" instead; having 300 10-line methods doesn't
concern me nearly as much as 10 300-line methods. Breaking code apart simply
because you want a smaller file means you have a crappy text editor. Break
code apart when it's logically doing more than it should be doing, or is
poorly encapsulated.

This is, to a limited degree, an issue I have with mocking in unit tests:
while great in theory (test components individually and trust that your
interfaces/contracts/whatever ensure that A and B independently working mean
that in production they will work together), in practice the two components do
have a dependency on each other, and it's exceedingly difficult to reproduce
production issues because of that fake isolation. How many ways have you seen
an external API fail? Have you reproduced those failure modes in your mock,
and have you written tests against each one of those different modes? Maybe
it's the industry I'm in (payments), but in a world made of edge cases,
effectively squaring the number of scenarios I have to test is not a pleasant
experience.

/edit - forgot my footnote :) [1] In practice, a user controller will
hopefully tend to always operate on the currently-authenticated user, rather
than passing around a userId by whatever means. However I've seen the latter
done, and this exact kind of problem is what ensued.

~~~
alisnic
I don't think I agree with the fact that every time you isolate a piece of
logic you add a layer. But I don't think I have a solid argument for that yet
:)

You do make a good point though that in having many small objects is easy to
mess with external dependencies. But in my case, the testing always suggested
if I had done something like that, and I improved. If an object becomes hard
to test in isolation, then something has been done wrong.

~~~
Firehed
Oh, I don't think isolating logic necessarily adds a layer, unless you go
overboard with isolating it. I think we're probably caught up on the subtle
differences between encapsulation and abstraction.

There seems to be a point where "I can't figure out what's happening here
without an IDE" starts to happen, which is a) relatively subjective and b)
sometimes a necessary side-effect. I only care when it's hard to test
something in isolation when it's actually used in isolation. A private method,
for example, exists only to be used by another component; it often makes
little sense to test independently, and when it does it's easy to wrap in a
public call that asserts that it's being run by a unit test.

But I think a lot of this is theoretical code purity vs prioritizing business
goals. All else being equal I'd love a perfectly-factored codebase, but at the
end of the day I have to target high-friction, high-confusion areas of the
codebase to refactor because those are the ones that get in the way of
building stuff for our customers, or cause the most production bugs.

------
jeswin
My experience has been:

\- A file (not a pattern) is the most effective unit of code organization and
navigation.

\- Any design, which can be split into multiple files in a logical and
consistent manner is maintainable.

A few years back, I might have cared about the issues mentioned in the
original post. Looking back, many of them were just personal preferences.

~~~
alisnic
> A file (not a pattern) is the most effective unit of code organization and
> navigation

This can be misinterpreted in many ways.

------
alisnic
So far, I got a lot of feedback here and on my blog, which I didn't expect
(also the frontpage). There seems to be a general attitude of "why
complicating things from day 1?" towards the article I wrote. While I do agree
that there is no need to isolate things from the start, I do not see the
complications at all.

The cost of creating a file and moving behaviour there is exactly 0, but the
benefits are there from day 1.

~~~
Firehed
Every extra file I have to look through when programming is an added cost;
abstractions add more than just CPU overhead. The highest-priced EC2 instance
on Amazon is under $5/hr; you won't find a decent programmer whose time is
worth anywhere near that little.

This may be relatively minor on a small project, but adds up quickly. It's
extra overhead when onboarding new developers, more time spent debugging
(deeper stack traces, etc.) more time simply waiting for grep to run. I'll
argue that you actually spend more time refactoring as well, since you're
encouraged to spend more time on the upkeep of this over-abstraction for the
sake of "clean code" rather than getting real work done.

Probably more important is that you don't know your six-months-from-now
problems now, you know them in about 5.9 months. I've wasted a lot of time
building the wrong abstraction layers because I made bad assumptions about our
future problems. At first, I thought that was a failure of my programming
abilities; I've come to realize that was my failure of my psychic abilities.

This isn't an attack on you or your style, merely a few observations about
what kind of effect it would have on my own work. If it works well for you,
awesome! However, it seems like most of this would only be really effective by
v3 or so of a product: v1 answers "is this useful and worth investing more
in", v2 covers "ok, let's turn this into a real product", and v3 is that giant
scary internal rewrite (that goes great thanks to your now-high unit test
coverage) that creates what you would have made from day one if you actually
knew what you needed.

------
darrencauthon
It all comes down to testing to me. If the dev is writing a test first,
questions about where things go are answered pretty quickly. If the tests
become painful or awkward, you're probably doing too much in the code.

I think the common Rails response to painful tests is to stop testing. I've
heard talk of the "diminishing returns" of testing, or debates of what
deserves to be tested. I was once responsible for maintaining many Rails 3
apps that I did not write, and the story was all the same: Either there were
no tests, or there were simple tests around simple logic, or there were simple
tests covering one or two paths through some _really_ complex logic. Once they
felt the pain from their tests, they just stopped writing tests. What else are
they going to do... challenge the "Rails Way?"

And this is the community that is supposedly known for their testing. Having
used Sinatra or Rails for almost all of my web work for a while, let me tell
you: The state of testing in the common Rails app is no different than I've
seen anywhere else.

~~~
amalag
This is a very good point. If you consider from the point of view of what
should be tested, then I think what the author proposes could be quite
natural. We don't want to duplicate and overlap with Rails tests. For me "what
to test" is the hardest question.

------
programminggeek
I've been working on the Obvious Architecture, which seeks to decouple your
app from both the database and your web framework. Treating Rails or Sinatra
or whatever as just a delivery mechanism frees you from a lot of of the
potentially bad design decisions that your web framework made about how you
should structure your app, the worst offender of this is how rails implies the
use of ActiveRecord. You should use Rails for what it's good at - web stuff
(routing, asset management, view engine integration).

I've got an example Obvious twitter close/status update app in progress at
<https://github.com/RetroMocha/obvious_status>

Obvious is still super new and I'm working on documentation and examples. You
can read more about Obvious at <http://obvious.retromocha.com>

~~~
TylerE
This sounds rather...performance killing.

Too much abstraction is _way way worse_ than too little, as well.

~~~
tinco
No it isn't.

Too much abstraction can always be refactored down to less abstraction. (Too
much abstraction means that there's abstraction that is unnecessary, thus it
can be cut away.)

Too little abstraction means your code is too closely coupled. When the code
is fresh and small, then ofcourse it's not hard to abstract out a method to
DRY something up. But when your code is big and old this will quickly approach
infeasability, which is the reason many projects fail.

I would go as far as saying that the science of software engineering exists
because your statement is false :P

~~~
andybak
So you're saying that the arrow of refactoring generally points in the
direction of less abstraction?

That directly contradicts my experience where most refactoring introduces
abstractions _when they are needed_.

~~~
ncallaway
My interpretation was that he was saying the arrow of refactoring is
<i>easier</i> when refactoring in the direction of less abstraction.

I suspect he agrees that generally the arrow points in the direction of
<i>more</i> abstraction. This would be the problem he thinks exists.

Not sure where I stand on either side of this; just trying to clear up what I
saw as a miscommunication.

~~~
tinco
Yes. Usually when programming one encounters problems that are solved
elegantly by introducing abstraction. The parent argues that doing this too
eagerly results in a big mess that should be avoided by being cautious with
abstraction. I argue the converse, being too cautious with abstraction can
result in a big mess and you should be eager to abstract.

Of course there is a golden path in the middle, but it is closer to my side ;)

------
alexborbely
MVC framework is just a delivery mechanism and your business logic should be
outside of it. Your business logic should have no idea about web, console,
controllers and routes, it should contain information only about your domain
objects and rules. You should be able to extract your application logic and
make it available on console, web or mobile just by changing the delivery
mechanism. This is the original explanation from <http://cleancoders.com> (Bob
Martin's website) [http://www.cleancoders.com/codecast/clean-code-
episode-7/sho...](http://www.cleancoders.com/codecast/clean-code-
episode-7/show)

~~~
virmundi
I tend to find this dream rather fevered. A controller knows about the model.
It knows about the view. It acts as a conduit between itself, the view and
model. In the web, the models are not really event based. Some platforms have
tried this, but it tends to not work out well at scale. So ultimately the
controller tends to get a lot of business logic embedded because it's
difficult to put that logic somewhere else.

The problem with this is that the controllers are very much view-centric. Take
.NET for example. There are forms. Logically this is the controller. Events
happen, handlers manipulate domain objects and create responses, the result
goes to the view. The problem is that even .NET makes a divide between
WinForms and WebForms. Entirely different namespaces. Thus your controller is
view bound and blocked.

Now you might want to push the logic from the controllers down into the domain
objects. This is fine. But, as other talked about with testing, you will have
to make sure that you're not doing too much with your domain objects. So
you'll probably start to use interfaces and constructors for the domain
objects to abstract away view details. You might even use callbacks (another
type of interface).

So you're feeling pretty good now. But eventually this type of configuration
become heavy. It also might not mesh with your system. I'm not sure about
Rails, but Hibernate makes is rather hard to IoC objects it makes. It's really
hard to do that by having Spring create your instance variables returned by
Hibernate.

In the end, the difficulty of making domain objects smart and loosely coupled,
but still coupled to other components while using an ORM makes the whole
attempt for naught. Thus you will circle back and put the business logic in
the controller or some controller adjacent code that is closely coupled with
the view.

~~~
mattgreenrocks
The whole point of attempting this is to allow these difficulties to force you
to decompose the system further than you would using the larger building
blocks of MVC.

About six months into working with Rails full time, I began wanting a third
place for my logic. Controllers were suboptimal, and models weren't always
right. Coming from a native apps, I felt restricted. I later learned that
POROs were well and good. But I doubted myself because the Rails way says
little.

~~~
virmundi
I'm good with POROs (assuming Plain Old Ruby Objects). I'm a huge fan of
Domain Driven Design. My issue with the parents is that without going into
what many Rubyists would call Java-land, one would find it hard to separate
concerns. Also, I don't agree that an MVC can truly be fully decoupled from
the V. This is why one seldom has one MVC to rule them all.

------
stiff
I think the importance of the Single Responsibility Principle is much
overestimated by many people and introducing terms like "single-responsibilty
objects" for what are essentially simple procedures is symptomatic of this.
There are many properties of classes one can consider beneficial, but they
might in fact conflict with each other to some extent, especially when carried
out to extremes like in this case, so what you want is to find a good
compromise. The "principle" itself is rather a very fuzzy guideline, so I wish
it wouldn't be taught to people since it seems to do more harm than good to
their understanding good OO design.

On another note, I wonder to what extent Rails usage of the Active Record
pattern makes "models" so clumsy in large applications. It works great to a
certain point, but maybe in large applications the Data Mapper pattern (not to
be confused with the DataMapper library) would help to isolate a large aspect
of what currently goes in a model while keeping a nice OO interface:

<http://www.martinfowler.com/eaaCatalog/dataMapper.html>

~~~
alisnic
> it seems to do more harm than good to their understanding good OO design.

Can you provide an example?

~~~
stiff
I heard this principle referenced in the context of objects such as presented
in the post quite a few times already, for example there was a heated
discussion here:

<http://news.ycombinator.com/item?id=4013807>

I explained back then why I consider this not object oriented at all. Here I
just wanted to say that if you imagine a spectrum of all the possible
breakdowns of a problem into classes, with one huge class at one end and a
breakdown so granular you have a separate class for each operation of the
system on the other end, there is a point where making the classes have less
responsibilities and having more of them is no longer beneficial to the
overall design, because you start to violate many other basic principles, for
example the "objects" don't really encapsulate any data anymore. I think
finding the sweet spot here is the kind of skill that is learnt with
experience and not easily passed on with words, hence the wording of the
principle is so fuzzy.

------
bstar77
My problem with fat models is exactly what the author describes, data really
needs to be separated from the logic. Since all of my current projects are
service oriented, I generally abstract into gems, albeit with some minor
overhead. That has proved to be the cleanest approach for me. Just throwing
code into /lib never seemed to help the organization/readability problems.

------
stiff
This seems to be a new trendy design pattern: wrap a single method with a
class and claim you just discovered a pinnacle of object-oriented design since
you respect the "Single Responsibility Principle"...

~~~
alisnic
> claim you just discovered a pinnacle of object-oriented design since you
> respect the "Single Responsibility Principle"...

I never said that, why all the hate? I stated pretty clear that this is my
approach on things, and I never pretended it to be the "right" one.

~~~
mattgreenrocks
Don't sweat it! People tend to adopt different subsets of OOP and then
proclaim all other subsets are wrong. Example: large-scale objects that are
heavily procedural vs lots of smaller grained objects that adhere to SRP.

I've written a lot of code in both styles, and determined that SRP is a good
force on my designs.

------
gavingmiller
I think someone needs to plug how awesome the Destroy All Software screencasts
are (referenced in the post.) They are well worth their value, and have made
me a better developer: <https://www.destroyallsoftware.com/screencasts>

~~~
alisnic
Totally agree.

------
why-el
I can't help but see the irony in the fact that the Rails community is slowly
moving back to app directories similar to those we found in Java's Spring and
others. I guess people from the Enterprise world had a point.

~~~
alisnic
There was a quote in Ruby Rogues podcast which said that "Ruby people use
factories a lot, they just don't call them like that"

------
jpallen
We use a very similar set up in a large Node.js app I'm working on. It works
very well for the most part, but we're definitely still figuring out the best
way to split things up.

For example, say we want to work with a 'user' model, and we need to do basic
CRUD stuff. One argument would say that we should have a module for each
action: creating users (registering, hashing password, sending out welcome
email, etc.), deleting users (cleaning up their data, removing them from the
db), and for authentication, profile updates, and so on. The downside to this
is that we end up with a dependence on the underlying data structure across
lots of modules. If we want to change the property 'username' to 'login', or
something more complex, we have a lot of modules to change. We also need to
understand how a user is created before we can understand how they are updated
or deleted, which means we need to have the CreateUser module in our head
while we're working on UpdateUser or DeleteUser.

Perhaps this problem is worse in our set up because we have only very thin
'models', which are basically just Mongoose wrappers around MongoDB, and so
our UpdateUser models is still quite heavily coupled to our database.

All things considered, I've found the schema described in this article is
better than any alternatives that we've tried. Perhaps we're just floating at
the equilibrium point where either more or less abstraction would make the
code more complicated. Certainly we refactor as soon as we can when a single
module gets too complex. As always, pragmatism rules.

------
aurelianito
Given code like this:

    
    
        class RatingsController < ApplicationController
        
          def create
            success = RatesBusiness.add_rating(params[:business_id],
                                               params[:score],
                                               current_user)
            render json: {success: success}
          end
        end
    

Wouldn't it be better something like this (just to adapt to rails)?

    
    
        class RatingsController < ApplicationController
          logic = RatesBusiness
        end
    

Or, even better

    
    
        class ApplicationController < Controller
          def initialize(logic):
            // do something 
          end
    
          def create:
            // do something with the logic
          end
        end
    

And adding some routes for each logic.

But, then, you could just extract the route from the logic name (and get the
rails controller back, that now it would be called "Logic")

My point is that extracting code just for the fun of it seems worthless. What
is the rationale in this case?

~~~
gavingmiller
What you're talking about looks like (is...) dependency injection. Rails is a
response to the Java FactorySingletonWidgetFactoryClass culture, so the
question is does DI benefit Rails, or simply allows us to pontificate about
our code? Which is what DHH is arguing against.

In my experience DI has been a great pattern for writing testable code, so do
you have any resources relevant to DI in Rails, and why it (DI) is more
beneficial than the OPs argument of a Service based architecture? (other than
a simple google search like I did?)

~~~
aurelianito
I am just pointing out that refactoring the code he proposed, removing
duplicate code, we arrive again at the Rails controller he does not like.

I see duplicate code all over this "service based architecture".

~~~
gavingmiller
I mostly agree, with the exception of the initializer, something I haven't
seen put to much use in controllers. The main thrust of my question was, do
you have any links/examples for a DI based rails controller? Other than your
"in theory" refactoring?

~~~
aurelianito
I do have examples of similar things but I cannot disclose the code as it was
written in closed applications. For instance, I am guilty of redoing poorly
the java servlet API using servlets.

------
tieTYT
As far as I can see, he's advocating a MVC approach. The controller is used
for handling View input/output. The Model is where the data and business logic
is supposed to go. For some reason most people think the Controller is where
the business logic is supposed to go, but when you research The MVC Pattern,
that's not how it's described.

If you don't believe me, let me give you a little history. The MVC Pattern was
created because of this problem: "I made this cool app with a fat client UI
and now the higher ups want a way to view it from a web page. This is so
freaking hard because the UI and the data and logic are all mixed together.
How could I have planned ahead for this?"

MVC is the answer. You take the View and the Control, make them specific to
the UI and then you put your UI agnostic code (this includes data AND business
logic) in the Model. That way, when the higher ups want a new UI, you just
have to create new View and the Controller code.

~~~
benrhughes
MVC is great for managing the display layer, but it doesn't really cover how
to manage your domain logic. IME, MVC scales best when the "model" is a
logicless DTO created by a business layer.

For small scale apps this is overkill, and you can squash persistence and
business logic into the model. But for some complex apps you really benefit
from a multi-layer approach.

------
sakopov
It's a pretty good practice in MVC. I don't work with Rails, but in ASP.NET
MVC, i put controller logic behind IBusinessTask generic interface which
defines a type for input and output of its single `Execute` method. Any
implementation of this interface would call the Business tier to perform its
task. These implementations are then injected into controller via property or
constructor injection.

------
LaGrange
Bah! Poppycock! My domain objects are also logicless, and I keep all my
enterprise stuff in PL/SQL. Also, TDD is for people who can't do formal
methods.

~~~
alisnic
sounds fancy, can you elaborate?

~~~
codewright
Yes, for $250/hour Corp-to-corp billing.

~~~
fruchtose
Wow! Sounds expensive, must be worth it.

------
dzuc
What to do about callbacks in the models?

~~~
amalag
I suppose we don't want to test the fact that Rails does a callback, we would
want to test the callback logic. We would test our domain class and then test
if the model class has a callback.

------
agopaul
Well, basically this is the approach of Application Service pattern, isn't it?

~~~
alisnic
I guess, but I am not informed enough to make a well-grounded statement about
this. I guess this is Don't-keep-your-logic-in-delivery pattern.

------
lucian303
"I look at Rails as web presentation layer, it makes my app work in the
browser."

Yes! YES! Not just rails. Any web app regardless of language or framework.
Thank you for writing this. It's what Richard C. Martin (Uncle Bob) calls the
lost years ... and about 15 of them. Reading books on good object oriented
design from 1993 sheds more light on how to build proper systems (that just
happen to have their delivery mechanism as the web).

The interface should be able to be changed. Right now, this is very difficult
in most frameworks (I do PHP but Uncle Bob uses Rails as his example ... it
really doesn't matter) and it's quite sad. This isn't a philosophical or
abstract discussion. This is about how to build software the right way.

You're not building a web app. You're building an app whose delivery mechanism
happens to be the web.

See Uncle Bob's talk: <http://vimeo.com/43612849>

Period.

