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.
I think part of the problem is that if you start with the naive way, you get some bad habits and start looking at your code through the lens of ActiveRecord - fat models, persistence, concerns, etc.
If your models were just plain ruby objects that validate your data for you, your code is able to do more interesting things - like multiple persistence mechanisms, multiple delivery mechanisms. Oh and they can be incredibly fast to test from day one.
If you use a tool like Obvious (which I wrote), you can TDD from day one and still build things fast, while maintaining a good structure.
I don't think everyone assumes that at all. Most discussions on DCI have been about "in big applications, this would work better" instead of "you should do this every time". Some of the supposed benefits aren't even seen in smaller chunks of code. Discussions on modules and SRP have not been absolutist either.
People have said things akin to your last paragraph: for simple projects there's nothing really wrong with the normal approach. The problem starts when it grows. The whole "movement" of Rails refactoring only began after people started having trouble moving large codebases around.
The guidance thing would be interesting. A collection of situations when things start to go weary, code smells. The thing is you can't do "when this happens, do this" because the "do this" part is the many solutions people have proposed.
However I think people have those code smells a bit down already and generally know what a controller should look like. The placement and structure of the domain logic is what is bugging.
Bingo. Having 80 files to move logic out of the controller because of 'design principle' is almost worse than having that code in the controller in the first place.
Apply based on reality, balance based on your needs. No articles or dogma required.
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.
If you do TDD, I don't see how this slows you down when you are isolated from the start. (Rails takes ~1 second to boot in the beginning).
And you are right, this was not intended to be presented as refactoring pattern, although it qualifies as one, this is how I manage code in my projects from day 1. And I do agree with you that is not always reasonable to do that, but I just feel more benefits rather than penalties doing it from the start.
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.
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.
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.
> 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.
Your AuthContext library sounds like a very unusual solution to this problem. Yes, there should be a single, unduplicated piece of code that restricts users' ability to edit each others' usernames. But why does that code need to know where the user is in the software? Surely admin users can always edit other users' names regardless of whether they're in the admin panel, and the question of whether or not to show the UI for editing usernames is a front-end concern. It sounds like an unusual and unnecessary tight coupling between the front-end and the back-end.
> 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
I'd argue that both are more or less equally harmful symptoms of the same problem: code that isn't clean. There are a couple of paragraphs about this in Clean Code that I like:
> At the same time, many developers fear that a large number of small,
> single-purpose classes makes it more difficult to understand the
> bigger picture. They are concerned that they must navigate from class
> to class in order to figure out how a larger piece of work gets
> accomplished.
> However, a system with many small classes has no more moving parts
> than a system with a few large classes. There is just as much to
> learn in the system with a few large classes. So the question is: Do
> you want your tools organized into toolboxes with many small drawers
> each containing well-defined and well--labeled components? Or do you
> want a few drawers that you just toss everything into?
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.
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.
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.
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.
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.
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.
The do it because it feels right (or don't do it because it feels wrong) mode of thinking is self-indulgent. When an app becomes larger and more involved so do the tests.
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).
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
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 ;)
Too much abstraction or indirection will certainly lead you down that path, but that is not really what I would advocate at all. With Obvious the goal is to make it feel obvious what the system is doing. No big complicated inheritance systems. Instead it uses a bit of indirection via dependency injection and reflection to make the data plugs swappable. Nothing too crazy or complicated.
It doesn't hurt performance in any noticeable way. Your database plugs are passed into the actions. The plugs are designed to do only what you want them to do, so you can tune them to your hearts content. In fact, I would argue it is far easier to tune a query in Obvious than it is in a traditional ORM which hides the query from you.
This is done via simple dependency injection, which is pretty standard in many other languages outside of ruby.
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...
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.
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.
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.
Take a look at giotto, a python web framework that tries to do exactly what you're proposing: separation of model, view, and controller so that they are completely decoupled: https://github.com/priestc/giotto
I agree. I based the Obvious Architecture around Uncle Bob's blog posts Clean Architecture and Screaming Architecture. It's taken months of testing, building and thought, but I think it's come out pretty nice on the other side. Both the delivery mechanism and the external persistence mechanisms are separate and pluggable from the app code itself.
You can go in the app/ directory of a project and read the file names of your actions and entities and know right away what the system does. Testing is fast because it never hits a database. Test doubles and mocking are easy to build and don't require a 3rd party lib. Data traveling between the app and the external systems is validated via contracts. It ends up being the most enjoyable TDD experience I've ever had.
For example, last night I put together an example "twitter" status update app in obvious. I got all the entities, actions, and contracts written TDD in about 3 hours. Today I'll be adding a delivery mechanism and probably filesystem and mysql plugs. Would be easy to write an api or console app and plugs for mongo, postgres, or whatever. Code is here: http://github.com/RetroMocha/obvious_status
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:
ActiveRecord is certainly a huge part of the problem and it is the first thing to decouple from your models. However there is a wonderful benefit to making since action objects - you can cleanly do dependency injection without side effects on the rest of your service. That makes testing them much cleaner and more obvious. The other really cool thing you could do is wire actions together with different sources. That could make things like data migrations between different data sources trivial or adding caching to smaller slices of your system easy.
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:
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.
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.
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"...
I think what the OP is talking about is just a ruby reimagination of the Operation pattern. Most sufficiently complicated apps that implement undo / redo use "Single Responsibility Objects" to encapsulate individual user operations. Each one has a "do" method and an "undo" method, and little else. In a web service, you don't have to worry about undo server-side, so using this pattern is more an option than a necessity!
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.
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
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.
I have been saying this for so long. Now we just have to wait for the Ruby community to understand why people do Dependency Injection and we'll be getting somewhere.
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.
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?)
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?
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.
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.
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.
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.
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.
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.
"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.
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.