155

In my Rails app, I use Rubocop to check for problems. Today it gave me an error like this : Assignment Branch Condition size for show is too high. Here's my code :

def show
  @category = Category.friendly.find(params[:id])
  @categories = Category.all
  @search = @category.products.approved.order(updated_at: :desc).ransack(params[:q])
  @products = @search.result.page(params[:page]).per(50)
  rate
end

What does this mean and how can I fix it?

chad_
  • 3,749
  • 2
  • 22
  • 22
THpubs
  • 7,804
  • 16
  • 68
  • 143

1 Answers1

161

Assignment Branch Condition (ABC) size is a measurement of the size of a method. It is essentially determined by counting the number of Assignments, Branches, and Conditional statements. (more detail..)

To reduce ABC score, you could move some of those assignments into before_action calls:

before_action :fetch_current_category, only: [:show,:edit,:update] 
before_action :fetch_categories, only: [:show,:edit,:update] 
before_action :fetch_search_results, only: [:show,:edit,:update] #or whatever

def show
  rate
end

private

def fetch_current_category
  @category = Category.friendly.find(params[:id])
end

def fetch_categories
  @categories = Category.all
end

def fetch_search_results
  @search = category.products.approved.order(updated_at: :desc).ransack(params[:q])
  @products = @search.result.page(params[:page]).per(50)
end
chad_
  • 3,749
  • 2
  • 22
  • 22
  • 2
    Great thanks. Now the code looks a lot readable but isn't it making the file large? More code? Is it good? – THpubs Jun 20 '15 at 14:39
  • 5
    Less code if you need those variables in other actions. – chad_ Jun 20 '15 at 14:40
  • I am getting the same on this method: # Draw the ball into this device context def draw(dc) dc.setForeground(color) dc.fillArc(x, y, w, h, 0, 64 * 90) dc.fillArc(x, y, w, h, 64 * 90, 64 * 180) dc.fillArc(x, y, w, h, 64 * 180, 64 * 270) dc.fillArc(x, y, w, h, 64 * 270, 64 * 360) end I don't seem to be able to preserve the code block layout here!!! What is going on here? There are no assignments, no branches, and no conditionals here at all!!!! – Lord Alveric Apr 01 '17 at 21:32
  • You have assignments implicit where you're multiplying the numbers. I'd take them and move them to constants so you're not re-evaluating the same arithmetic in those calls. I'm not sure if this will fix your linter's feedback but it would certainly clean it up a tiny bit. :) – chad_ Apr 12 '17 at 17:11
  • You could assign them all in one line like: `RQ,RH,RT,RW=[90,180,270,360].map { |i| i * 64 }` – chad_ Apr 12 '17 at 17:21
  • i can see this work for cases you'd use instance objects, but for those that don't how do you solve high ABC ? – olleh Jul 04 '17 at 10:45
  • Generally it's a matter of properly abstracting your objects and considering the interface you're presenting. It's a pretty vague question you've asked, but given an example project I could present some ideas for refactoring away the complexity of your classes. Sorry I can't give a more concrete suggestion without a more concrete example. – chad_ Jul 19 '17 at 15:29