On logic in a Rails app
alisnic.github.com
alisnic.github.com
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.
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.
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.
Apply based on reality, balance based on your needs. No articles or dogma required.
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.
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.
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.
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.
> 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.
> 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
> 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?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.
- 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.
This can be misinterpreted in many ways.
The cost of creating a file and moving behaviour there is exactly 0, but the benefits are there from day 1.
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.
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.
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
Too much abstraction is way way worse than too little, as well.
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
That directly contradicts my experience where most refactoring introduces abstractions when they are needed.
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.
Of course there is a golden path in the middle, but it is closer to my side ;)
This is done via simple dependency injection, which is pretty standard in many other languages outside of ruby.
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.
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.
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
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:
Can you provide an example?
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.
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.
I've written a lot of code in both styles, and determined that SRP is a good force on my designs.
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.
class RatingsController < ApplicationController
def create
success = RatesBusiness.add_rating(params[:business_id],
params[:score],
current_user)
render json: {success: success}
end
end
class RatingsController < ApplicationController
logic = RatesBusiness
end
class ApplicationController < Controller
def initialize(logic):
// do something
end
def create:
// do something with the logic
end
end
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?
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 see duplicate code all over this "service based architecture".
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.
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.
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.