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

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.

Answer 2

I don't use rubocop, but I did test the following with this data:

data = <<FILE
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
12.55.123.255 - - Hello
FILE

using String#gsub! and Enumerable#select (Reports AbcSize of 3)

def task_2(str)
  str.each_line.select do |x|
    # Without named groups 
    # x.gsub!(/\A([\d+\.\d+]+).*(?<=\[)(.*)(?=\]).*(?<=\s)((?:\/\w+)*?)(?=\s).*\z/m,
    # '\2 FROM \1 TO \3')
    x.gsub!(/\A(?<ip>[\d+\.\d+]+).*(?<=\[)(?<date_time>.*)(?=\]).*(?<=\s)(?<address>(?:\/\w+)*?)(?=\s).*\z/m,
      '\k<date_time> FROM \k<ip> TO \k<address>')
  end
end


task_2(data)
# => ["23/Apr/2018:20:30:39 +0300 FROM 10.6.246.103 TO /test/2/messages", 
#      "23/Apr/2018:20:30:42 +0300 FROM 10.6.246.101 TO /test/2/run"]

Here we are using String#gsub! with a pattern replacement, which will return nil if no replacement is made thus rejecting it from Enumerable#select.

Similar solution, although likely less efficient, using String#match, Enumerable#map, and Array#compact (Reports AbcSize of 7.14)

def task_2(str)
  str.each_line.map do |x|
    match = x.match(/\A(?<ip>[\d+\.\d+]+).*(?<=\[)(?<date_time>.*)(?=\]).*(?<=\s)(?<address>(?:\/\w+)*?)(?=\s)/)
    "#{match['date_time']} FROM #{match['ip']} TO #{match['address']}" if match
  end.compact
end

Here we are using String#match to extract the match data and then confirming a match and outputting the desired format if there is a match. A string that does not match will output nil and thus we compact the Array to remove the nil values.

Another option could just be to scan the whole String all at once and break out the matching groups: (Reports AbcSize of 5)

def task_2(str)
  str.scan(/^([\d+\.\d+]+).*(?<=\[)(.*)(?=\]).*(?<=\s)((?:\/\w+)*?)(?=\s).*$/)
    .map {|a| "#{a[1]} FROM #{a[0]} TO #{a[2]}"}
end

Can make the last one as low as 2.24 via

 def task_2(str)
  r = []
  str.scan(/^([\d+\.\d+]+).*(?<=\[)(.*)(?=\]).*(?<=\s)((?:\/\w+)*?)(?=\s).*$/) do |ip, date_time, address | 
    r << "#{date_time} FROM #{ip} TO #{address}"
  end
  r
end

Answer 3

Any time I run into a ABC too high (or similar complexity/length warnings), I'm pretty quick to just chop the method up. Your readability, testability, and maintainability almost always improve.

The quickest way is to yank out the body of a loop or conditional into a new method. Repeat as needed until you can read each method in one breath.

Similarly, if you've got big complex conditionals/loop constructs, pull that out to a new method as well.

Combining those two strategies enough times will reduce any method into roughly two method calls. That might be a bit overzealous in some cases...but it's never too far.

Here's one way you could apply that strategy to your code:

def task_2(str)
  result = []

  str.each_line do |x|
    ip, datetime, address = parse_line(x)

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

  result
end

def parse_line(x)
  ip = x[/^.* - -/]
  datetime = x[/[\[].*[\]]/]
  address = x[/T .* H/]
  return [ip, datetime, address]
end

s =<<EOF
123.123.123.999 - - [2009-12-31 13:13:13] T www.google.com H"
456.456.456.999 - - [2009-12-31 13:13:13] 404"
678.678.678.999 - - [2009-12-31 13:13:13] T www.amazon.com H"
EOF

puts task_2(s)

Produces the output:

2009-12-31 13:13:13 FROM: 123.123.123.999  TO:  www.google.com
2009-12-31 13:13:13 FROM: 678.678.678.999  TO:  www.amazon.com

If you wanted to go even farther, you could pull the body of each_line out to a new method, process_line, etc. And if you created a class, you could avoid the messy (to my eye) multi-value returns.

Answer 4

This is a problem where it is conventient to use named capture groups.

R = /
    (?=                       # begin a positive lookahead
      (?<ip>.*\s-\s-)         # match the string in a capture group named 'ip' 
    )                         # end positive lookahead
    (?=                       # begin a positive lookahead
      .*                      # match any number of characters
      (?<datetime>[\[].*[\]]) # match the string in a capture group named 'datetime'
    )                         # end positive lookahead
    (?=                       # begin a positive lookahead
      .*                      # match any number of characters
      (?<address>T\s.*\sH)    # match the string in a capture group named 'address' 
    )                         # end positive lookahead
    /x                        # free-spacing regex definition mode

def task_2(str)
  str.each_line.with_object([]) do |s, result|
    m = str.match(R)
    result << m[:datetime][1..-2] + ' FROM: ' + m[:ip][0..-4] +
              'TO:' + m[:address][1..-3] unless m.nil?      
  end
end

str =<<_
123.123.123.999 - - [2009-12-31 13:13:13] T www.google.com H"
456.456.456.999 - - [2009-12-31 13:13:13] 404"
678.678.678.999 - - [2009-12-31 13:13:13] T www.amazon.com
_
task_2 str
  #=> ["2009-12-31 13:13:13 FROM: 123.123.123.999 TO: www.google.com",
  #    "2009-12-31 13:13:13 FROM: 123.123.123.999 TO: www.google.com",
  #    "2009-12-31 13:13:13 FROM: 123.123.123.999 TO: www.google.com"] 

The regular expression is conventionally written as follows.

R = /(?=(?<ip>\A.* - -))(?=.*(?<datetime>[\[].*[\]]))(?=.*(?<address>T .* H))/

Notice that where I have spaces here I had whitespace characters (\s) when writing the regex in free-spacing mode. That's because in free-spacing mode spaces are stripped out before the expression is evaluated. Alternatively, spaces can be preserved in free-spacing mode by enclosing them in character classes ([ ]).