我应该重构这个深度嵌套的ruby方法吗？

下面的Ruby代码是应该分解成更小的方法还是应该保留"原样",因为如果分解它会碎片过多？我们不确定应该采用什么样的正确方法.我们知道使用单个测试的小方法通常是最佳实践,但是在这种情况下,这样做似乎会使这些代码过于"薄"并大大降低可读性.这类代码在这方面的最佳实践是什么？

if $?.success?
    puts "Running git fetch"
    `git fetch`
    if $?.success?
        puts "Running git reset --hard origin/#{mbranch}"
        `git reset --hard origin/#{mbranch}`
        if $?.success?
            puts "Running git merge origin/#{branch} --no-ff -m \"STAGED:#{ticket} - #{title}\""
            mergeres=`git merge origin/#{branch} --no-ff -m "STAGED:#{ticket} - #{title}"`
            if $?.success?
               `git log -n 1 | grep "STAGED:#{ticket}"`
               if $?.success?
                  ret=true
                  if resjs eq "yes"
                     puts "Entering reservejs directory..."
                     `cd /#{dir}/zipcar-main/zipcar acs/packages/zipsite/www/reservations/reservejs`
                     if $?.success?
                        puts "Running git fetch on reservejs"
                        `git fetch`
                        if $?.success?
                           puts "Running git checkout master on reservejs"
                           `git checkout master`
                           if $?.success?
                              puts "Running git reset --hard origin/master on reservejs"
                              `git reset --hard origin/master`
                              if $?.success?
                                 puts "Running git merge origin/#{branch} on reservejs"
                                 `git merge origin/#{branch} --no-ff -m "STAGED:#{ticket} - #{title}"`
                                 if $?.success?
                                    `git log -n 1 | grep "STAGED:#{ticket}"`
                                    if $?.success?
                                       b=Dir.chdir("#{firstdir}")
                                       return true
                                    end
                                 end
                              end
                           end
                        end
                     end
                  end
               end
            else
               puts "#{mergeres}"
            end
         end
      end
   end

Alex Wayne.. 5

你的结构尖叫需要一些迭代.而不是深层嵌套的条件,为什么不是一个步骤数组,而你只是在步骤失败时中止处理.

def do_stuff

  # array of [cmd, output] pairs.
  steps = [
    ["git fetch",                          "Running git fetch"],
    ["git reset --hard origin/#{mbranch}", "Running git reset --hard origin/#{mbranch}"],
    # more steps and feedback labels
  ]

  steps.each do |step|
    cmd, feedback = step # step[0] is the command, step[1] is the feedback
    puts feedback
    return false unless system(cmd).success?
  end
end

或者您可以使用哈希数组,甚至可以让您根据需要扩展"步骤"的定义方式.

steps = [
  {
    cmd: 'git fetch',
    msg: 'Doing a git fetch, hold onto your butts!'
  },
  # more step hashes...
]

这使得步骤的线性顺序更具可读性.而且将来如果你必须在某个地方插入一个命令,你可以这样做而不会愤怒地咒骂.

根据你想要走多远,你可以制作一个Step封装每一步的课程!

steps = [
  Step.new(
    cmd: 'git fetch',
    msg: 'Doing a git fetch, hold onto your butts!'
  ),
  # more step instances...
]

steps.each do |step|
  step.run!
  return false if step.failed?
end

您甚至可以使每个步骤接受一个块,以便围绕该步骤运行任何自定义逻辑.

Step.new(
  cmd: 'git fetch',
  msg: 'Doing a git fetch, hold onto your butts!'
) do |result|
  # run some ruby code with result of command before next step
end

并且你可以创建一个运行各种步骤的类!

runner = StepRunner.new(
  Step.new(
    cmd: 'git fetch',
    msg: 'Doing a git fetch, hold onto your butts!'
  ),
  # more step instances..
)

runner.run!

if runner.success?
  puts 'all steps complete!'
else
  puts "failed with error: #{runner.error}"
end

但那已经进入了过度杀伤区.这取决于你想要这个系统的灵活性.如果这是一种常见的模式,但步骤的类型和数量各不相同,那么您可能需要更多的抽象.如果这是一个孤立的事件,你可以在更简单的一面做一些事情.