Authentication and session creation

Question

My session controller has a method for creating new user session. According to Rubocop's output there is 'Assignment Branch Condition' metric is too high [15.17/15].

    def create
      agent = Agent.find_by(login: params[:session][:login])
      if agent && agent.authenticate(params[:session][:password])
        log_in agent
        redirect_to dashboard_url
      else
        flash.now[:danger] = 'Invalid login or password'
        render 'new'
      end
    end

log_in method:

    def log_in(agent)
      session[:agent_id] = agent.id
    end

I've extracted method for processing unsuccessful logins to reduce ABC size:

  def create
    agent = Agent.find_by(login: params[:session][:login])
    if agent && agent.authenticate(params[:session][:password])
      log_in agent
      redirect_to dashboard_url
    else
      unsuccessful_login 'Invalid login or password'
    end
  end

  private
    def unsuccessful_login(message)
      flash.now[:danger] = message
      render 'new'
    end

1. Is it appropriate to make new method I'm going to use only once in this case?
2. Is there any guideline when it's suitable to extract method?

Answer 1

1. It is not technically wrong. In OOP languages you can encounter this sort of thing (a private method apparently only used once) especially in loops. Or if you want a specific part of the algorithm to not be overrideable, etc. Generally spoken however, I see no reason to do it in your example. The code was clear and now has way more clutter.

2. Whenever you repeat yourself in a class, whenever you use copy paste from another class, is most oftentimes an indication that you could abstract the code to a set of smaller functions / objects.

3. With regards to the tool reporting a high ABC rank: those tools do not IMHO replace common sense. You should never sacrifice readability and conciseness for the sake of a better score. This is a case where your code quality tool might have too rigid settings, or perhaps there is more code in the inspected scope that is better suited or in more need of refactoring.