Refactor 和项目大小有什么关系？

看完描述，我第一个反应是：如果 var_a, var_b, var_c 都是临时变量，并且方法 a 和方法 b 只负责处理这 3 个临时变量。那么把方法 a 和 b 放在当前这个类里的意义是？

这种情况下我会把计算出 var_a 和 var_b 的那些变量拿出来放到一个新的类里，变成成员变量，然后在新的类里面组织这些成员变量的方法，把方法 a 和 b 移到这个类里面。

楼主的描述还是比较模糊，比如说计算出 var_a 的变量有哪些，我们根本没法知道。其实讨论这样的问题把真实代码拿出来最好。

说个题外话，有时候我蛮提倡过度设计的。

首先，“过度设计”好过“没有设计”，这两者相较起来，前者起码说明程序员在思考，说明这些代码是被管理着的。

其次，“完全没有设计”是一个极端，“过度设计”是另一个极端。 程序员要两个极端都到过，似乎才更好把握“中间”在哪里，才能更好的权衡。和“过度设计”深入地打过交道，才能更清楚地知道它到底长什么样，好像不用一开始就拒绝它吧，你说呢？

其实我觉得“让表达更清晰”这一个理由就足够让我动手对代码进行封装了。而“能不能被重用”有时候更像是个结果：短小的代码更有机会被重用，也更容易被重用。

有许多程序员认为把实现完全展开，“平铺直叙”的代码更“直观”，而另一部分程序员认为合理封装的代码更“直观”。如果对于评价一件事物好和坏的标准不一样，那好像确实只能得出不同甚至相反的结论。

另外，保证接口的逻辑正确这种事情是由提供接口的人来做的，而不是使用接口的人。我觉得这里职责就有点没分清楚了。

临时变量的作用域是局部的，如果某个方法里需要不断地访问这个临时变量，那么这个方法就必定会被拉得很长。临时变量很多时候是拆分方法的一个障碍。

短小的方法总是比长方法容易阅读和维护，这是我们写代码时的价值观之一，也是对方法进行拆分的动机。如果连这一点都不认同，那么确实，拆分方法没有意义，消除临时变量也没有什么意义了。

另一方面，一个对象首先是由一组逻辑上相互关联的成员变量组成的，然后再附带了一些针对这些成员变量进行操作的方法。当一个方法里出现许多临时变量的时候，程序员就要分析对这些临时变量进行操作的代码应该是属于当前对象的职责，还是属于另一个新对象的职责。

如果参与某个临时变量的计算过程的整块代码使用的全部都是该类内部的成员变量或实例方法，说明这块代码本就属于当前类的职责，那么这个临时变量可以很容易的被抽出来变成该类自身的一个方法

#重构前
  def method_a
    variable_a = @instance_variable_a + get_b
    variable_b = variable_a - CONSTANT_B
    puts variable_a + variable_b
  end

  def get_b
    @instance_variable_c  * CONSTANT_A
  end
#重构后
  def method_a
    puts variable_a + variable_b
  end

  def get_b
    @instance_variable_c  * CONSTANT_A
  end

  def variable_a
    @instance_variable_a + get_b
  end

  def variable_b
    variable_a - CONSTANT_B
  end

如果参与临时变量计算过程的代码块使用的主要是传进来的参数与该类本身的某一两个成员变量或者实例方法，那么这块代码很可能属于另一个还未被发现的对象。我就直接搬重构手法“使用方法对象替换方法”的例子吧：

#重构前
class Account
  def gamma(input_val, quantity, year_to_date)
    important_value1 = (input_val * quantity) + delta
    important_value2 = (input_val * year_to_date) + 100

    if (year_to_date - important_value1) > 100
      important_value2 -= 20
    end

    important_value3 = important_value2 * 7
    important_value3 - 2 * important_value1
  end
end
#重构后
class Account
  def gamma(input_val, quantity, year_to_date)
    Gamma.new(self, input_val, quantity, year_to_date).compute
  end
end

class Gamma
  def initialize(account, input_val, quantity, year_to_date)
    @account = account
    @input_val = input_val
    @quantity = quantity
    @year_to_date = year_to_date
  end

  def compute
    important_value3 - 2 * important_value1
  end

  def important_value1
    (@input_val * @quantity) + @account.delta
  end

  def important_value2
    result = (@input_val * @year_to_date) + 100
    if (@year_to_date - important_value1) > 100
      result -= 20
    end
    result
  end

  def important_value3
    important_value2 * 7
  end
end

这两种情况无论哪一种都是在消除临时变量。简单地说，就是临时变量的出现就意味着有某个对象的职责未被封装好。

对于你说的后面那半句，其实我想聊的还多一些。

我们去 KFC 点餐的时候，如果是一家店的常客，当服务员对我们说“套餐 A 今天有优惠，只要 18 元”，我们马上就可以做出选择“要还是不要套餐 A”。

但如果我们是第一次来这家店，那么肯定要先搞清楚套餐 A 里到底有些什么东西，比如中可乐、小薯条、奥尔良鸡腿堡……

再假如，我们是第一次吃 KFC，不知道中可乐有多大、小薯条有多小、奥尔良鸡腿堡里面到底有哪些料，可能又要花时间弄明白这些名字到底是什么意思。

这个例子里就已经有 3 次概念的封装了。我们人类认识事物的方式以及交流方式就是这样：先弄明白细节，然后把有共同点的细节归纳成一个抽象的概念，再用抽象的概念来进行高效的交流。上面的例子当中，“套餐 A”这个概念最抽象，也最高效；“中可乐”、“小薯条”这些概念抽象层次低一些，交流起来也相对麻烦一点；第三种情况明显就最麻烦，估计是服务员最不愿意遇到的情况了吧。

在代码里高效的交流也是这样，如果程序员读一段代码，仅仅从名字上就能知道这段代码在干嘛，那他根本没必要去看具体的实现，一句抽象的话能说清楚的事情就不用把细节展开来啰嗦。

class GetOrderItems
  def get
    items = []
    response = throttle(2){ @client.list_order_items(@amazon_order_id).parse }
    items += Common::Helper.array(response['OrderItems']['OrderItem'])
    while(next_token = response['NextToken']) {
      response = throttle(2){ @client.list_order_items_by_next_token(next_token).parse }
      items += Common::Helper.array(response['OrderItems']['OrderItem'])
    }
    items
  end
end

对于上面这段代码，首先从类名 GetOrderItems 和 public 方法名 get 就可以知道这个类的职责，如果你在阅读代码时满足于此，那就没必要翻开 get 方法来读它的实现。所以封装很重要，封装之后要起一个容易看懂的名字，否则还是要打开方法看具体的实现，所以可以说命名决定着封装的成功还是失败。

当你想了解 get 方法的时候，可能又发现 get 方法的实现还是有些复杂，也许这个例子不容易看出来这个 get 方法有多难读，但是我把它再整理一下：

class GetOrderItems

  def initialize
    #其它代码
    @items=[]
  end

  def get
    fill_items
    fill_next_page while more_items?
    @items
  end

  private

  def fill_items
    @response = throttle(2){ @client.list_order_items(@amazon_order_id).parse }
    collect_items
  end

  def fill_next_page
    @response = throttle(2){ @client.list_order_items_by_next_token(@next_token).parse }
    collect_items
  end

  def more_items?
    @next_token = @response['NextToken']
  end

  def collect_items
    @items += Common::Helper.array(@response['OrderItems']['OrderItem'])
  end
end

你看这个 get 方法的结构会不会清晰得多？同样也是仅用方法名就表达了它所做的事情，起码描绘清楚了这段代码的结构，把抽象层次更低的细节封装了起来。如果读到 get 方法里这 3 句代码已经足够让你明白它在做什么，就没必要再往下挖，看更具体的实现。

在楼主那个例子里，如果代码确实只有楼主贴出来的那部分那么简单，我个人其实还是会把变量赋值的细节封装起来，虽然从变量名能看出来意思，但是这个赋值总是让我忍不住要去读它后面的表达式。但是如果团队里有人坚持要这么写，我觉得也不是什么问题——前提是代码只有楼主贴出来的那部分那么简单。

其实我觉得后半部分这个话题蛮好。关于整理代码的意识，很多时候——就像你说的——人们习惯于总结出一些非此即彼的“百分之百”的标准，更不愿意在具体的场景下具体分析，引用一段话：

并不是所有的临时变量都是不好的，也并不是所有参数多于 1 个的方法就是有问题的。这里是故意把“大量临时变量”中的“大量”给省去了，把“多个参数”写成了“2 个参数”。因为不在具体的上下文当中，“大量”和“多个”根本没个标准。

这里想要表达的是，所有的 bad smell 都是一种反模式，所谓“模式”就是很容易被人发现的一种有规律的定式，一旦遇到就要动脑子分析，和具体的规模无关。但很多程序员倾向于总结出非此即彼的固定标准，因为具体问题具体分析太费脑子。所以：临时变量既然不可能完全消除，就完全不需要消除；方法参数既然不可能永远不超过 1 个那就可以是 5, 6, 7, 8 甚至 10 多个；方法行数既然不能全部减少到 5 行那就说明 100 来行也是没问题的。最终失去了对 bad smell 的警觉。

而训练有素的程序员会把这些模式当作“触发器”，时时刻刻警觉着分析这些现象在当前的情况下到底算是个问题还是算正常情况。训练有素的程序员知道很多东西不是非此即彼，不可以一概而论。

但是很多情况下我们似乎都倾向于追求和谐的统一，因为那看起来更不需要思考。只是采用这种“不需要思考”的方式写代码造成结果却往往是需要更多的思考，因为混乱的代码结构维护起来一定比条理清晰的代码费脑得多。

讨论起写代码的方法就比较兴奋，忍不住地想表达自己的想法，可能比较啰嗦，还会跑题，见谅。

是的。

关于你说的第 2 点，我知道有人会有疑问。

其实我考虑得比我上面说的还要多一些，把方法抽出来只是第一步。

根据贴出来的代码，他这里的 param 我估计是个临时的 hash 变量，类似这样：

def action_name
  param = {}
  # ...其它代码...
  param[:type_id] = if params[:thirdCategory].present?
    params[:thirdCategory]
  elsif params[:secondCategory].present?
    params[:secondCategory]
  elsif params[:topCategory].present?
    params[:topCategory]
  end
  # ...其它代码...
  SomeClass.new(param).do_something
end

假如真的是这样的话，我最终的代码会是：

def action_name
  # ...其它代码...
  SomeClass.new(param).do_something
end

private

def param
  {
    :type_id => type_id,
    :other_keys => other_values
  }
end

def type_id
  if params[:thirdCategory].present?
    params[:thirdCategory]
  elsif params[:secondCategory].present?
    params[:secondCategory]
  elsif params[:topCategory].present?
    params[:topCategory]
  end
end

当然，如果代码确实只是楼主贴出来的那部分那么简单，那么你说的没错，完全可以直接使用临时变量来存储这个表达式的结果，用变量名来表明意思。我第一步这么做只是在我的意识当中，“临时变量”是个 bad smell（关于这一点如果需要讨论的话，咱们还可以继续展开），我会对临时变量有所警惕，要不要消除临时变量我都会加以判断——如你所说：任何百分百的东西都是伪命题。

而关于第 3 点，代码结构怎么组织的问题，我觉得只要是个正常的对象，组织代码的方式其实都差不多：mixin、继承、组合。除非像 Rails 的 Controller 这样，把单次生命周期内上下文不相关的成员变量强塞在一起组成一个对象，每次实例化 Controller 对象之后又仅仅只调用它的其中的一个 public 方法——通过这种方式来隔离作用域，这样想要组织代码确实比较头疼。

param[:type_id] = params[:thirdCategory].presence || params[:secondCategory].presence || params[:topCategory].presence

v.s

param[:type_id] = type_id

谁短？你没有给出所有代码，没有上下文我只能写成这样。下一步我估计这个 param[:type_id] 都可以省掉（我注意到你这个 param 没有 s），直接改为在所有的地方调用 type_id 方法。

而 type_id 方法无论是写成

def type_id
  if params[:thirdCategory].present?
    params[:thirdCategory]
  elsif params[:secondCategory].present?
    params[:secondCategory]
  elsif params[:topCategory].present?
    params[:topCategory]
  end
end

还是写成

def type_id
  params[:thirdCategory].presence || params[:secondCategory].presence || params[:topCategory].presence
end

对使用的人或者阅读的人来说都没有影响。

至于 params.values_at(:a, :b, :c).find(&:present?) 反而读着比楼主的还费解一些，当然封装之后也关系不大。

基本上稍微有点复杂的表达式都应该封装成方法，无论是为了重用，还是仅仅只为了可读性。

好像很多程序员经常会进入一个误区，就是在短短的几行代码具体的实现上纠结所谓的“优雅”。 其实想要“优雅”封装成一个方法，起好名字就完了。

def type_id
  if params[:thirdCategory].present?
    params[:thirdCategory]
  elsif params[:secondCategory].present?
    params[:secondCategory]
  elsif params[:topCategory].present?
    params[:topCategory]
  end
end

需要用的地方只要调用 type_id 就好，方法实现得怎么丑陋都没关系。代码就是要在命名上让别人一眼看明白，具体实现本就该被封装起来，里面的代码你们爱怎么折腾怎么折腾，只要逻辑不会出错就随便你改。

好奇你们现在不搞 Ruby 了么？