Rails controller to manage EC2 instances I did a small sample application for an Interview. Since I didn't got accepted I would like your opinion on how to improve it.

The aux methods are private.
I have a usage_common to remove duplication in the usage and usage_instance methods
add_item_hash is used because the format written in the database is different from the used in the API response

The start stop and status actions are used to simple operations in EC2 instances

Maybe I should've broken it in services? one for usage, one for processes and one for EC2.

Should send the params already in the format for database and thus eliminate the methods build_usage_hash and build_process_hash 

https://github.com/mabranches/monitor_api/blob/master/app/controllers/instances_controller.rb

class InstancesController &lt; ApplicationController
  before_action :set_instance, only: [:show, :update, :destroy]

  def usage
    usage_common(:get, (Time.now - 24.hours).utc.iso8601, Time.now.utc.iso8601)
  end

  def usage_instance
    usage_common(:get_instance, params[:id], (Time.now - 24.hours).utc.iso8601, Time.now.utc.iso8601)
  end

  def processes_instance
    items = ProcessList.get_instance(params[:id])
    process_items = {}
    items.each do |item|
      process_items[item['instance_id']] = item
    end
    render json: process_items
  end

  def create
    usage = build_usage_hash params
    process = build_process_hash params

    Usage.create(usage)
    ProcessList.create(process)

    render json: [usage, process], status: :created
  rescue =&gt; e
    Rails.logger.error("Was not possible to update instance data: #{e.message}")
    render json: { error: e.message }, status: :unprocessable_entity
  end

  def processes
    items = ProcessList.get
    process_items = {}
    items.each do |item|
      process_items[item['instance_id']] = item
    end
    render json: process_items
  end

  def stop
    @params = params
    ec2_action(:stop, 'stopped')
  end

  def start
    @params = params
    ec2_action(:start, 'started')
  end

  def status
    @params = params
    ec2_instance do |ec2|
      @status = ec2.state.name
    end
  end

  private

  def usage_common(method, *args)
    items = Usage.send(method, *args)

    result = {}
    items.each do |item|
      add_item_hash(result, item)
    end
    render json: result
  end

  def ec2_instance
    instance_id = @params[:id]

    return render json: {}, status: :unprocessable_entity unless instance_id
    ec2 = Aws::EC2::Resource.new(region: 'us-west-2').instance(instance_id)
    @status = 'unknown'
    begin
      yield ec2
    rescue =&gt; e
      Rails.logger.error("Was not possible to execute action #{@params[:action]}: #{e.message}")
    end
    render json: { instance: instance_id, status: @status }
  end

  def ec2_action(method, status)
    ec2_instance do |ec2|
      ec2.send(method)
      @status = status
    end
  end

  def add_item_hash(result, item)
    result[item['instance_id']] ||= Hash.new { |h, k| h[k] = [] }
    result[item['instance_id']]['mem'] &lt;&lt; item['mem']
    result[item['instance_id']]['disk'] &lt;&lt; item['disk']
    result[item['instance_id']]['cpu'] &lt;&lt; item['cpu']
    result[item['instance_id']]['usage_time'] &lt;&lt; item['usage_time']
  end

  def build_usage_hash(params)
    instance = params[:instance]
    {
      instance_id: params[:id],
      cpu: instance[:cpu],
      mem: instance[:mem],
      disk: instance[:disk],
      usage_time: Time.now.utc.iso8601
    }
  end

  def build_process_hash(params)
    instance = params[:instance]
    {
      instance_id: params[:id],
      process: instance[:process].try(:values) || []
    }
  end
end
I see two things wrong with this controller. 


It's too 'fat'. You're doing way too much in one controller.
It's not RESTful. 


In this case, those two problems are much the same. Without reading too much into the code, it looks like you could break that controller up into 3 controllers, UsageController, InstanceController, Instance::PowerController. I'm not certain about the last one, you may find the Instance's power state not to be a RESTful resource.

Here's an example: https://stackoverflow.com/documentation/ruby-on-rails/4373/method-on-a-controller-that-s-not-part-of-the-default-five#t=201608110000585779419

As well though, I'd say the building of the hashes should be encapsulated by a model.