Skip to content

Commit

Permalink
Service obkect post
Browse files Browse the repository at this point in the history
  • Loading branch information
andreacfm committed Nov 7, 2015
1 parent 45bc3a0 commit cb60df0
Show file tree
Hide file tree
Showing 2 changed files with 280 additions and 1 deletion.
279 changes: 279 additions & 0 deletions _posts/2015-11-07-fat-models-or-controllers-services-to-the-rescue.md
@@ -0,0 +1,279 @@
---
layout: post
title: "Fat Models? Fat Controllers? Services to the rescue!!!"
date: 2015-11-07 15:55
comments: true
---
MVC has become the "de facto" standard for building web (and not only web) application. Enforcing the separation of concerns between presentation and business logic, MVC has become a sort of "common land" that makes easy to follow the application flow in a cross language manner.

## Models, Controllers and Business ...

Why are we talking about Fat Controllers and Models?

Mvc leaves to developers the freedom to choose where to place and elaborate the business logic of the application. When we talk about business logic we refer to a set of classe (library) that actually encapsulate the actions/methods that represent the application identity. If for example we are coding an e-commerce app we will need logic about what products I sell, how and where do I configure these products etc... All this logic should be the core app library that exposes an API to be used through an MVC pattern.
When all or part of this logic is coded into controllers and/or models we talk about fat controllers and models.

Why is this an issue? Having fat controllers and models says that our business logic is sparsed all around and that when the application grows will be more and more difficult to maintain and refactor our codebase.

At the end we are talking about coding a flexible codebase respecting some well known rule like the **single responsibility principle**. Something that is fat in terms of methods and dependencies will for sure brake this basic rule messing things around.

Before starting with the code let's make something clear about models and controllers.

* a **MODEL** represents the instance of a defined entity that makes part of the application domain. Any method called on the model instance should be oriented to expose some aspect of the instance itself.

* a **CONTROLLER** is an endpoint. It should only be concerned about receiving a call, giving the application business logic a place to execute what needs to be executed and render the view using the provided data.

### Example. A Rails basic app.

Let's say we are coding an application that has a model called **Task** with some basic attributes:

{% highlight ruby %}
# Task.rb
class Task
include Mongoid::Document
field :name
field :description
end
{% endhighlight %}

If we want to persists a task we normally implement the logic into a method in a TasksController:

{% highlight ruby %}
#tasks_controller.rb
class TaskController < ApllicationController

def create
task = Task.new params[:task]
if task.save
........
end

end
{% endhighlight %}

This is quite basic and works well even if, from a theorical point of view, adding an **ORM** as mongoid in our example extends the model responsibility over what he should know about the world around. But since **ORM** makes our life much more easy and productive we are happy to accept this as a minor side effects.

Now let's go a bit further. A new story in the next sprint says that once a Task has been created we must enqueue a background job to notify some other app and even send a notification to every user that joined our account. We think this is easy enough and we add this logic to our controller:

{% highlight ruby %}
#tasks_controller.rb
class TaskController < ApplicationController
def create
@task = Task.new params[:task]
if @task.save
_enqueue_job
_notify_taks_to_users
........
end

def _enqueue_job
.....
end

def _notify_taks_to_users
.....
end

end
{% endhighlight %}

This 2 new methods should already "smell" cause we are adding business logic to the controller that should not be aware of what happens when a Task is created. Our controller is getting **FAT**. But, at the end, this was easy and we closed the story very fast!! Is friday and a cold beer is waiting for us.

The next sprint complicates a bit our life. We are asked to add a module to the admin area of our application where the users with SUPERADMIN role should be able to manage tasks as well in any application account scope.

We add an /admin/tasks_controller to our application cause we cannot reuse the other controller cause they are base on different acl rules, etc....

{% highlight ruby %}

#/admin/tasks_controller.rb
class TaskController < ApllicationController

def create
@task = Task.new params[:task]
if @task.save
_enqueue_job
_notify_taks_to_users
........
end

def _enqueue_job
.....
end

def _notify_taks_to_users
.....
end
end

{% endhighlight %}

Here we have an issue. We are duplicating the same business logic in 2 different points. Even if we are lazy coders we see that this is not a safe way. We could create a module to be included but more than a solutions looks more like hiding the problem.

Looking at the code we see that the 2 extra methods we added belongs to a particular phase of our application. Any time a task is created we must enqueue a job and send out a notification. So maybe this duties belongs to the Task model itself. Great! We think we are done. We reset the 2 controllers to the basic state:

{% highlight ruby %}
#tasks_controller.rb
#/admin/tasks_controller.rb_
class TaskController < ApllicationController

def create
task = Task.new params[:task]
if task.save
........
end

end
{% endhighlight %}

and we rely on the Task model callback to accomplish our post creation duties:

{% highlight ruby %}
# Task.rb
class Task
include Mongoid::Document
field :name
field :description

after_create :_enqueue_job, :_notify_taks_to_users

def _enqueue_job
.....
end

def _notify_taks_to_users
.....
end
end
{% endhighlight %}

After running our test suite ee already note a downside. Our code works but now anytime a Task instance is persisted the 2 callbacks gets fired. Even if we create the instance from a spec factory the callbacks are fired .... We do not like this because it adds unwanted noise to any Task creation but we think we will survive. The controllers are skinny, the model is getting FAT ... but we decide we are still going well.

And then comes the pain! We are asked to send a different notification to the user that created the task (actually the current_user) but not if the user is the SUPERADMIN.

How we can do that? The business logic is now completely managed by the Model that does not know nothing about the concept of Task creator. We could add this info as a field to the Model but is not something that is not required to be persisted.

Btw there is always a solution.....

{% highlight ruby %}

# Task.rb
class Task
....
attr_accessor :creator
after_create :_enqueue_job, :_notify_taks_to_users, :_notify_creator
.....

def _notify_taks_to_users
notify anyone
end

def _notify_creator
notify @creator unless @creator.superadmin?
end

end
# in both the tasks controllers
def create
task = Task.new params[:task]
task.creator = current_user
if task.save
........
end

{% endhighlight %}


We can do this so why is this bad?

* we are adding a not required external dependencies to a Model. From now any time a Task instance is persisted the creator accessor must be provided. Over that the Task model also needs to be aware of the User role ....
* we complicate our test suite factory. We must provide a creators of differents roles.
* we open a way to add more and more dependencies and we will end with a model that plays more and more roles.

### Services to the rescue

We could easily solve the problem encapsulating the business logic that stays behind the creation of a Task into his own class. A class that makes part of the application library and is responsible just for creating tasks.
We reset the model to the basic implementation. Then we add a service class:

{% highlight ruby %}

#/app/services/create_task_service.rb
class CreateTaskService
attr_accessor :current_user, :task_params

def initialize(args={})
@current_user = args.fetch(:current_user, params) do
raise ArgumentException 'Current user is missing!'
end
@task_params = args.fetch(:params, {})
end

def run!
task = Task.new task_params
_after_create(task) if @task.save!
task
end

def _after_create(task)
_enqueue_job(task)
_notify_taks_to_users(task)
end

def _enqueue_job(task)
.....
end

def _notify_taks_to_users(task)
notify anyone
notify current_user unless current_user.superadmin?
end

end

{% endhighlight %}

What about our 2 controllers?

{% highlight ruby %}

#tasks_controller.rb
#/admin/tasks_controller.rb_
class TaskController < ApplicationController

def create
begin
@task = CreateServiceTask.new(
current_user: current_user, params: params[:task]
).run!
rescue => e
#handle
end
end

end

{% endhighlight %}

What are the benefits here?

* CreateTaskService is a brick of our app core library. It just does one thing, creates a task, exposing a clear API. It requires a user, the task params and returns a task instance. Being devoted to this particular task is completely acceptable that it has dependencies like a Background job engine or a MailService.
* Testing a plain ruby class is easy and fast.
* Testing the business logic in completely isolation from the framework.
* Approaching new feature is easier because we can really focus on the logic coding and then use it inside of the framework. A similar approach to coding a rest api.
* We slowly build the core application that exposes all the methods we need. When we add new features we will look into this API as when we look for endpoints in a REST api. More services will means more endpoint to rely on.
* We do not mess things up in our spec factories. A task_factory will give us back a Task instance following our attributes configuration. No callbacks. No tricks.

Does implementing a Service layer slow development down?

In my experience no. Once the pattern is clear and respected by any team member it can also increase the team productivity. Is much more easy to code on expectations when you know that your teammate is developing a class that will require some params and will give you back what you expect. You do not need to wait the real code as you do with rest API.
Over that, when the application grows, you will have a set of self documented class that can help anybody to better understand the application logic and find the right method to use.

#### Notes:

* using the name **services** with a **run** method is just a convention. At the end we are talking about plain classes. The only important thing to keep in mind is that a class is done to perform a task and that you must be able to rely on the methods signature and returns.

Happy coding!!!



2 changes: 1 addition & 1 deletion s3_website.yml
Expand Up @@ -2,7 +2,7 @@ s3_id: <%= ENV['S3_ACCESS_KEY'] %>
s3_secret: <%= ENV['S3_SECRET'] %>
s3_bucket: andreacfm.com
cloudfront_distribution_id: E1B2DZR9HI3XO2
s3_endpoint: eu-west-1
s3_endpoint: us-west-1

# Below are examples of all the available configurations.
# See README for more detailed info on each of them.
Expand Down

0 comments on commit cb60df0

Please sign in to comment.