Redefine what counts as a branch on ABC Size

[jafuentest]

I was thinking if it would be possible to not count getter methods as a branch. I know this has been asked before (can't find the issue now, but I de remember reading something about it) and I'm just wondering if it may be possible with a plugin instead of a configuration setting.

The reason for it is pretty simple, take this method for example

def search
  @posts = model.active.visible_by(current_user)
    .search(params[:q], skip: @skip)

  @posts = model.some_process(@posts, current_user)
  @posts = model.another_process(@posts, current_user)

  render 'pages/search/page', locals: { model: model }
end

It's ABC would be <3, 15, 0> = 15.3

Now, if instead of calling model and current_user every time, you assign their values to a variable you get:

def search
  the_model = model
  user = current_user
  @posts = the_model.active.visible_by(user)
    .search(params[:q], skip: @skip)

  @posts = the_model.some_process(@posts, user)
  @posts = the_model.another_process(@posts, user)

  render 'pages/search/page', locals: { model: the_model }
end

It's ABC would be <5, 10, 0> = 11.18

So the reason this bothers me is that the last method is considered the least complex one, but in reality it takes more time for someone to read through it.

Branches are defined as "an explicit forward program branch out of scope", I would say getter methods in ruby are not really "explicit" as they pretty much look like just any variable. But I'm aware that's highly a matter of personal opinion.

So wrapping up, is there a way to not count any getter method (defined for this purpose as any method that receives no arguments) as branch while calculating ABC? I had a look at AbcSizeCalculator class but I can't figure out where the node.type comes from in the branch?(node) method

[marcandre]

It is indeed tricky. As you realize, it's impossible to know if, in your example, model is just a attr_reader or a method that makes a web request synchronously, thanks to the dynamic nature of Ruby.

The AbcSize metric was tweaked extensively recently (#8037) and also we reduced some repeated conditional paths (#8408). I didn't add a setting for that last tweak as it would be plausible for Ruby to actually optimize this internally. In this particular case, Ruby can not optimize this (even if it is an attr_reader, as the instance variable @model can change at any time). So I don't think we can justify not counting it by default.

That said, we could discuss a setting to discount repeated send without arguments. In this example:

def foo
  model.do_something  # branch +2
  model.user.send_email if model.user.needs_invoice? # condition +1, branch +3 instead of +6
end

The first model would count as +1, but not the others, and only the first .user could be counted.

[jafuentest]

That said, we could discuss a setting to discount repeated send without arguments

Yeah, that actually makes more sense than not counting them at all

[marcandre]

@jonas054 I'd love your input on this and #8986

[jonas054]

@marcandre I have no objections to your idea about counting multiple instances of the same method call with no arguments as 1 within a method. Technically these method calls could be "branching out of scope" like any other. Nevertheless, it makes sense to view them differently as we continue to shoehorn this metric into the world of Ruby. 🙂

[marcandre]

PR #9053 is up