CODEWITHSUNDEEP
ruby-on-railsrubyrefactoringstyling
Angel Garcia
Angel Garcia

Reputation: 1577

Fixing repetitive methods in ruby

I'm writing an API in ruby on rails, and I find that my methods are extremely repetitive. This is how almost all of my methods look like. They all follow a similar structure; I just grabbed one of my methods and placed it here:

def create
  if current_user

    @object = Object.new(object_params)

    if @object.save
      render json: {
        status:  "SUCCESS",
        message: "object saved"
      }, status: :ok
    else
      render json: {
        status: "ERROR",
        message: "Could not save object"
      }, status: :unprocessable_entity
    end
  else
    render json: {
      status: "UNAUTHORIZED"
    }, status: :unauthorized
  end
end

Of course, not all of my methods create and save objects. But they all follow a similar pattern. first I check if the current_user is not nil. If it is, I render a JSON response:

if current_user
  # do something
else
  render json: {
    status: "UNAUTHORIZED"
  }

if the current user is found, and whatever the method has to run is successful, then I will render another JSON response:

if everything_went_well
  render json: {
    status: "SUCCESS",
    message: "Everything went ok!"
  }
else
  render json: {
    status: "ERROR",
    message: "There was a problem!"
  }
end

I'm pretty sure there is a way to abstract this sort of functionality. I've looked up tips online that have helped but in other parts of the project (i.e., models and views). Most of the tips I've seen online, however, aren't very adaptable to the code I have in my controllers.

Upvotes: 2

Views: 171

Answers (2)

Omid Kamangar
Omid Kamangar

Reputation: 5788

If all of you controllers look like that, then you can have a base controller, and have all other controllers inherit from that controller. This would make your code DRY.

So, your base controller would look something like this:

class API::BaseController < ApplicationController
  protected

  def check_permission
    return if current_user

    render json: {
      status: "UNAUTHORIZED"
    }, status: :unauthorized
  end

  def render_success
    render json: {
      status:  "SUCCESS",
      message: "object saved"
    }, status: :ok
  end

  def render_error
    render json: {
      status: "ERROR",
      message: "Could not save object"
    }, status: :unprocessable_entity
  end
end

Then in one of your controllers:

class API::OtherController < API::BaseController
  before_filter :check_permission

  def create
    @object = Object.new(object_params)

    if @object.save
      render_success
    else
      render_error
    end
  end
end

You can also move the before_filter part to the base controller, but that would be less flexible. With current setup, you have that filter only for some of your actions, say before_filter :check_permission, only: [:create].

But if you move the before_filter to the base controller, you will lose that flexibility.

Upvotes: 0

spickermann
spickermann

Reputation: 106972

You can use a before_filter for this. Note that when a before_action already rendered a view that the action itself will not be called anymore.

before_action :check_permission

def create
  @object = Object.new(object_params)

  if @object.save
    render json: {
      status:  "SUCCESS",
      message: "object saved"
    }, status: :ok
  else
    render json: {
      status: "ERROR",
      message: "Could not save object"
    }, status: :unprocessable_entity
  end
end

private 

def check_permission
  return if current_user

  render json: {
    status: "UNAUTHORIZED"
  }, status: :unauthorized
end

Upvotes: 3

Related Questions