2

Two Models: An Owner and a Dog:

owner.rb

class Owner < ActiveRecord::Base
  has_one :dog
end

dog.rb

class Dog < ActiveRecord::Base
    belongs_to :owner
end

And here is the schema:

schema.rb

ActiveRecord::Schema.define(version: 123) do

  create_table "dogs", force: true do |t|
    t.string   "name"
    t.integer  "energy"
    t.integer  "owner_id"
    t.datetime "created_at"
    t.datetime "updated_at"
  end

  add_index "dogs", ["owner_id"], name: "index_dogs_on_owner_id"

  create_table "owners", force: true do |t|
    t.string   "name"
    t.string   "energy"
    t.datetime "created_at"
    t.datetime "updated_at"
  end

end

Pretty simple setup.

I want an owner to take his dog for a walk. When the walk ends, the owner's energy will drop by 5, AND the dog's energy will drop by 20.

Clearly this walk_the_dog action/method, wherever it is going to live, is effecting two objects: an owner object and a dog object (and of course this dog object happens to be associated to this owner).

I don't know where to put this code. I know I could simply create an action within the owners_controller.rb, but that seems like a bad idea. It would look something like this:

owners_controller.rb

class OwnersController < ApplicationController
    def walk_the_dog
        @owner = Owner.find(params[:id])
        @owner.energy -= 5
        @owner.dog.energy -= 20   # this line in particular seems like bad OO design
        @owner.save
        @owner.dog.save
    end
    ...
 end

As I understand it, objects should only change state for themselves and shouldn't change the state of other objects. So this appears to be a bad idea because within the owner controller we are changing the state of not just the owner object, but the associated dog object as well.

I have read about services. It seems like walk_the_dog is an excellent case for a service, because services, as I understand it, allow interactions between objects and state changes for multiple objects. I just don't know how to do it/implement it.

Should there be a service object called walk_the_dog? Should their just be a file within a services directory with a bunch of service methods -- one of which is called walk_the_dog and the owners_controller.rb simply utilizes this method in it's controller? I don't know what the next step is.

Note: I can see someone saying "who cares if this breaks OO design. Just do it and if it works, it works." Unfortunately this is not an option. The application I am working on now followed that mindset. The application grew very large, and now maintaining it is very difficult. I want to get this situation down for the major redesign of the app.

Arslan Ali
  • 17,418
  • 8
  • 58
  • 76
Neil
  • 4,578
  • 14
  • 70
  • 155
  • 2
    I'd use a walk the dog service. But following OOP, it couldnt do stuff like `@owner.energy -= 5` you;d have to implement a `decrement_energy` method – apneadiving Jun 11 '15 at 16:24
  • @apneadiving any tips or suggested resources on how to implement services in a rails app? Do I have the concept of a service in a rails app correct? – Neil Jun 11 '15 at 16:37
  • 1
    http://slides.com/apneadiving/service-objects-waterfall-rails/live#/ – apneadiving Jun 11 '15 at 16:42

2 Answers2

1

Here are the few things that I would do if I were to refactor this code:

Writing numbers in your code is a bad thing, either you have defined them as constants like ENERGY_PER_WALK_FOR_DOG = 20 or a better way is to define a field in the table of Dog model. This way, it will be much better to manage and assign those values.

add_column :dogs, energy_per_walk, :integer, default: 20
add_column :owners, energy_per_walk, :integer, default: 5

I'd create a method in ApplicationController class:

def walk(resources = [])
  resources.each do |resource|
    resource.lose_walk_energy # you can refine it more!
  end
end

In the folder, app/models/concerns, I would write the following module:

module Walkable
  extend ActiveSupport::Concern


  # subtract energy_per_walk form the energy saved in db
  def lose_walk_energy
    self.energy -= self.energy_per_walk
    save
  end

end

And now, your method reduces to the following method:

def walk_the_dog
  @owner = Owner.find(params[:id])
  walk([@owner, @owner.dog])
end
Arslan Ali
  • 17,418
  • 8
  • 58
  • 76
  • thank you very much for your response. Follow up question: If the application scales then one couldn't continue putting methods in ApplicationController. Otherwise there could be dozens, or even a hundred methods in there! Where would you put those methods then when the application scales? So for your refactoring above: where would you put the method: walk when ApplicationController becomes too bloated? – Neil Jun 11 '15 at 16:42
  • 1
    We define those methods in `ApplicationController` that are shared between two or more controllers. If you so a lot of code shared between two controllers, then it means you need to make a new controller, and make those two controllers to inherit from it. – Arslan Ali Jun 11 '15 at 16:47
0

I would say that this should be a method in the Owner model. You also need to do both operations in one transaction, to ensure, that both models have been updated.

class Owner
  has_one :dog

  def walk_the_dog
    return false if dog.nil?

    transaction do
      decrement!(:energy, 5)
      dog.decrement!(:energy, 20)
    end
  end
end
Yury Lebedev
  • 3,985
  • 19
  • 26