Assignment Branch Condition size is too high

Question

I'm making method that takes multiline string (logs) and writes new strings to array.

def task_2(str)
  result = []
  str.each_line do |x|
    ip = x[/^.* - -/]
    datetime = x[/[\[].*[\]]/]
    address = x[/T .* H/]
    if !ip.nil? && !datetime.nil? && !address.nil?
      result << datetime[1..-2] + ' FROM: ' + ip[0..-4] + 'TO:' + address[1..-3]
    end
  end
  result
end

and I need it to pass rubocop analysis with default configuration, but it gives AbcSize 18.68/15 And I'm sure that because of if..end statement, but how can I rewrite it?

Log example:

10.6.246.103 - - [23/Apr/2018:20:30:39 +0300] "POST /test/2/messages HTTP/1.1" 200 48 0.0498
10.6.246.101 - - [23/Apr/2018:20:30:42 +0300] "POST /test/2/run HTTP/1.1" 200 - 0.2277

Answer 1

The ABC size is calculated by doing the following:

√(assignments² + branches² + conditionals²)

Let's first have a look at the assignments:

result = []
ip = x[/^.* - -/]
datetime = x[/[\[].*[\]]/]
address = x[/T .* H/]

This leaves us with 4 assignments.

Next up the branches. For this I have to mention that most of the operators are methods (thus count towards branches) for example 1 + 1 could also be written as 1.+(1) + is a method on an integer. The same applies for string[regex], which could be written as string.[](regex) [] is a method on strings. And !value which could be written as value.!@ !@ is a method on all objects.

With that out of the way let's count the branches.

str.each_line
x[/^.* - -/]
x[/[\[].*[\]]/]
x[/T .* H/]
!ip.nil? # counts for 2 (! and .nil?)
!datetime.nil? # counts for 2 (! and .nil?)
!address.nil? # counts for 2 (! and .nil?)
result << ...
datetime[1..-2]
ip[0..-4]
address[1..-3]
+ # 4 times in result << ... + ... + ....

This leaves us with 18 branches.

The last thing to count are the conditionals. Since Ruby uses short circuiting with the && and || operators they will count towards conditionals.

if
&& # 2 times

This leaves us with 3 conditionals.

√(4² + 18² + 3²) ≈ 18.68

Now that we have an understanding of where the number is coming from, we can try and reduce it. The easiest way to reduce ABC size is by reducing the thing with the largest number, since this number is squared. In your case these are the branches. You already spotted where the issue lies in the question.

if !ip.nil? && !datetime.nil? && !address.nil?
  result << datetime[1..-2] + ' FROM: ' + ip[0..-4] + 'TO:' + address[1..-3]
end

Could be simplified to:

if ip && datetime && address
  result << "#{datetime[1..-2]} FROM: #{ip[0..-4]}TO:#{address[1..-3]}"
end

Taking a total of 10 branches away. 3 times !something.nil? (which count for 2, since ! and .nil? are both counted towards branches) and 4 times +.

Leaving you with:

√(4² + 8² + 3²) ≈ 9.43

Answer 2

def task_2(str)
  result = []
  str.each_line do |x|
    ip = x[/^.* - -/]
    datetime = x[/[\[].*[\]]/]
    address = x[/T .* H/]
    if ip && datetime && address
      result << datetime[1..-2] + ' FROM: ' + ip[0..-4] + 'TO:' + address[1..-3]
    end
  end
  result
end

Having !variable.nil? is redundant. Basically, you are checking presence here, so #present? method would suffice, but any value that is not nil or false is considered false, so to be more idiomatic it is better to just use the form I used in the if statement. This solves ABS issue.