HiveBrain v1.2.0
Get Started
← Back to all entries
snippetrubyMinor

Coding style in Ruby format in an method and variable assign

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
formatmethodstylerubyandvariablecodingassign

Problem

Case 1_a:

zip_file = file.sub(directory, '').sub!(/^\//, '')
  zipfile.add( zip_file, file)


Case 1_b:

zipfile.add( 
    file.sub(directory, '').sub!(/^\//, ''),
    file)


Case 2 : If the String contains lots of parameters , I prefer write in an array.But I'm not sure to have a line break behind [ is better, or not

def run_remote_focus()

    self.setup_remote_focus()
    created_at=(Time.now.to_f*1000000).to_i.to_s

    settings=[
      "-i #{params[:remote_focu][:ip]}",
      "-u #{params[:remote_focu][:username]}",
      "-p #{params[:remote_focu][:password]}",
      "-s #{params[:RemoteFocu].values.join(':')}",
      "-m #{params[:af_type]}",
      "-t #{params[:remote_focu][:tester]}",
      "-n #{@remote_focu.id}",
      "-c #{created_at}"
    ]

    cmd=["ruby",
         "-I #{@rf[:lib]}",
         "#{@rf[:bin]}",
         " #{settings.join(' ')}"
         ]
    cmd_str=" #{cmd.join(' ').strip()} & "


Case 3 : I can not tell what is the problem, but it just seems weird

@rf={
     folder: "#{Rails.root}#{ENV["module_remote_focus_folder_path"]}",
     bin: "#{Rails.root}#{ENV["module_remote_focus_bin_path"]}",
     lib: "#{Rails.root}#{ENV["module_remote_focus_lib_path"]}"
     }

@rf={folder: "#{Rails.root}#{ENV["module_remote_focus_folder_path"]}",
     bin: "#{Rails.root}#{ENV["module_remote_focus_bin_path"]}",
     lib: "#{Rails.root}#{ENV["module_remote_focus_lib_path"]}"}

Solution

Case 1_a. Don't mix pure functions (sub) with destructive functions (sub!). Whenever possible use pure methods:

zip_file = file.sub(directory, '').sub(/^\//, '')
zipfile.add(zip_file, file)


But can't you write simply?

zip_file = File.basename(file)


Case 1_b: I prefer to set variables. Giving names to things is good.

Case 2 : It's ok. I also prefer to write a comma at the last element so it can be freely reordered. I'd also change the structure of settings so it's easier to write:

settings = [
  ["-i", params[:remote_focu][:ip]],
  ["-u", params[:remote_focu][:username]],
  ...
].map { |opt, value| [opt, value].join(" ") }


Case 3 : Use the same indentation style that you used for array. Also, use the library facilities, don't manipulate paths as strings (extremely prone to bugs):

@rf = {
  folder: File.join(Rails.root, ENV["module_remote_focus_folder_path"]),
  ...
}

Code Snippets

zip_file = file.sub(directory, '').sub(/^\//, '')
zipfile.add(zip_file, file)
zip_file = File.basename(file)
settings = [
  ["-i", params[:remote_focu][:ip]],
  ["-u", params[:remote_focu][:username]],
  ...
].map { |opt, value| [opt, value].join(" ") }
@rf = {
  folder: File.join(Rails.root, ENV["module_remote_focus_folder_path"]),
  ...
}

Context

StackExchange Code Review Q#35400, answer score: 8

Revisions (0)

No revisions yet.