Executing a command using Net::SSH

Question

According to the local cop (rubocop), my method has too many lines.

lib/awesomelibrary/tunnelable.rb:4:5: C: Method has too many lines. [17/10]
 def tunnel_run(cmd) ...
 ^^^^^^^^^^^^^^^^^^^

Below is the method. How do I refactor it?

module AwesomeLibrary
 # Gives object the ability to execute bash commands on itself via ssh 
 module Tunnelable
 def tunnel_run(cmd)
 retries = 0
 code = nil
 Net::SSH.start(@tunnel_ip, @tunnel_username, keys: @tunnel_key_name, verify_host_key: false) do |ssh|
 the_channel = ssh.open_channel do |channel|
 channel.exec cmd do |ch, success|
 raise "could not execute command" unless success
 ch.on_data { |_c, data| print data }
 ch.on_extended_data { |_c, _type, data| print data }
 ch.on_request("exit-status") { |_ch, data| code = data.read_long }
 end
 end
 the_channel.wait
 end
 abort "#{cmd} returned #{code} !!" if code != 0
 rescue Net::SSH::ConnectionTimeout
 puts "Net::SSH::ConnectionTimeout"
 retry if (retries += 1) < 3
 end
 end
end

I'm using this is the library: Net::SSH 4.x.

• Flambino: You don't necessary need to refactor anything. Sure, it could probably be made prettier by one metric or another, but.. eh'. I'd be more concerned that it'll retry forever, since retry will start the method over, setting retries = 0 each time...
• Der Kommissar: As much as I hate to be "that guy", letting a script tell you that your method has too many lines is a terrible reason to refactor. Does it work? Does it make sense? Is it testable? Those are the important questions, not "how many lines does it have?".

Answer 1

In my opinion, your method does not have too many lines, but the line calling Net::SSH.start is too long.

My terminals and text editors are 80 characters wide, so I would split the long line into 2 lines

 Net::SSH.start(@tunnel_ip, @tunnel_username, keys: @tunnel_key_name,
 verify_host_key: false) do |ssh|

If you must reduce the number of lines, you can move some lines into new methods. The extra method calls will make spaghetti of the control flow, so the code will be difficult to read and understand, but it might satisfy your cop.

 def tunnel_run(cmd)
 retries = 0
 code = tunnel_run_command(cmd)
 abort ... if code != 0
 rescue Net::SSH::ConnectionTimeout
 ...
 end
 private
 def tunnel_run_command(cmd)
 code = nil
 Net::SSH.start(...) do |ssh|
 the_channel = ssh.open_channel do |channel|
 channel.exec do |ch, success|
 code = tunnel_run_channel(ch, success)
 end
 end
 the_channel.wait
 end
 code
 end
 def tunnel_run_channel(ch, success)
 code = nil
 raise ... unless success
 ch.on_data { ... }
 ch.on_extended_data { ... }
 ch.on_request("exit-status") { ... }
 code
 end

Beware that the private methods tunnel_run_command and tunnel_run_channel will pollute the method namespace in all objects that extend Tunnelable. Privacy in Ruby is by object, so the object inherits the private methods from Tunnelable, and Ruby allows the object to call those methods.

The names tunnel_run_command and tunnel_run_channel must not conflict with other methods of the object.