I have an action method in my Rails controller which filters an ActiveRecord model depending on the submitted GET parameters.
Rubocop complains that Assignment Branch Condition Size is too high: 20.62/15.

Is there a way to improve the following code?

@measurements = Measurement.all
@measurements = @measurements.where(sensor_id: params[:sensor_id].split(',')) if params[:sensor_id].present?
@measurements = @measurements.last_per_sensor(params[:last_per_sensor]) if params[:last_per_sensor].present?
@measurements = @measurements.last(params[:last]) if params[:last].present?

Some notes:

  • That's too much code for a controller method, it should be moved to the model.

  • The same name (@measurements) holds up to 4 different values. As a general rule, avoid variable rebinding: different values, different names.

  • More about how the filtering method could be written keeping it DRY: https://stackoverflow.com/a/4480285/188031

For example:

class Measurement < ActiveRecord::Base
  def self.filter(attributes)
    attributes.select { |k, v| v.present? }.reduce(all) do |scope, (key, value)|
      case key.to_sym
      when :sensor_id
        scope.where(key => value)
      when :last, :last_per_sensor 
        scope.send(key, value)

And in the controller simply:

@measurements = Measurement.filter(params.slice(:sensor_id, :last, :last_per_sensor))

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.