DRY-ing up code with some Helper Methods

Here is some recent work I did DRY-ing up my code. I had these two methods in one of my controllers.

class FightsController < ApplicationController
def new
        @monster = Monster.find_by(id: params[:monster_id])
        if my_monster?(@monster) 
            @fight = @monster.fights_challenged.build
            @monsters = Monster.others(@monster.id)
        else
            @fight = @monster.fights_defended.build
            @monsters = current_user.monsters
        end
    end

def create
        @monster = Monster.find_by(id: params[:monster_id])
        if my_monster?(@monster) 
            @fight = @monster.fights_challenged.build(fight_params)
            @monsters = Monster.others(@monster.id)
        else
            @fight = @monster.fights_defended.build(fight_params)
            @monsters = current_user.monsters
        end

        if @fight.save
            @fight.attack
            redirect_to monster_path(@monster)
        else
            flash[:alert] = @fight.errors.full_messages
            render :new
        end
    end 
end

This is for a web app that allows you to create monsters and have them fight each other. When you visited a monster's page the code would determine if it was your monster. If it was you could choose from a drop down and pick a different monster to fight (Meaning it gave you a list of every monster, except the one whose page you were on). If it wasn't your monster you could choose from a drop down of which of YOUR monster's you'd like to send to fight the one you are viewing (So in this case you got a list of every monster that your account was associated with).

So the code did a couple of things. It determined if you were visiting your monster or not, then it built a list of monsters, and built the appropriate fight object. It's a lot of code, and much of it repeats in both actions. So I needed to create some helper methods so that I only had the same code written once.

First off I removed the line of code that determined which monster's page you were nested on.

 before_action :get_monsters, only: [:new, :create]

    def set_monster
        @monster = Monster.find_by(id: params[:monster_id])
    end

That's one line of code removed. Next was the trickier part. I had to create a helper method out of the large chunk of code that generated the list of monsters and created the appropriate fight objects. The only difference between the two similar chunks of code was that the create action took in parameters. So when I placed the code in a helper method I needed logic to decide if I was on a create action or a new action.

This is what I came up with:

before_action :get_monsters, only: [:new, :create]

      def get_monsters
        if my_monster?(@monster) 
            @fight = if_challenger
            @monsters = Monster.others(@monster.id)
        else
            @fight = if_defender
            @monsters = current_user.monsters
        end
      end

     def if_challenger
         if params[:action] == "new"
             @monster.fights_challenged.build
         else
             @monster.fights_challenged.build(fight_params)
         end
     end

     def if_defender
         if params[:action] == "new"
             @monster.fights_defended.build
         else
             @monster.fights_defended.build(fight_params)
         end
     end

So now, both my original actions (new and create) called get_monsters which in turn decided if they needed to call if_challenger or if_defender.

Except if_challenger and if_defender looked very similar. Was there a way to make that one method instead of two so I could cut down on even more repeated code? There was, but it involved using the send method.

The send method takes a number of arguments. The first one it assumes is a method, and the rest are it's parameters. Here is what it would look like if I used a send method to dynamically create a setter method:

self.send("#{method}=", value) #is the same as: self.method= value

So, the one part that was different in the two methods had to be supplied to the send method. That means I had a variable that would be either "fights_challenged" or "fights_defended"

So I replaced is_challenger and is_defender with build_fight that made use of the send method, like this:

    def get_monsters
        if my_monster?(@monster) 
            @fight = build_fight("fights_challenged")
            @monsters = Monster.others(@monster.id)
        else
            @fight = build_fight("fights_defended")
            @monsters = current_user.monsters
        end
    end

    def build_fight(arg)
        if params[:action] == "new"
            @monster.send(arg).build
        else
            @monster.send(arg).build(fight_params)
        end
    end

Let's talk through this one more time. When get_monsters is called the method determines if it is nested in your monster's page or not. Then it builds the appropriate monsters array. And to prepare a form on the app it calls build_fight(arg). This second method checks the params[:action] to see what action the user is currently on and then uses the arg to build the @fight object dynamically.

The send method makes your code a bit trickier to read compared to most of ruby's eye friendly language, but it's very powerful in it's ability to work with changing arguments.