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

params[:thirdCategory] 这种会取到 ""空值

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

可以可以，666

又涨姿势了，这个知识点 guides 里面有吗

有的呀 presence

我的关注点是，简化如下代码，能用一行实现的事情为啥要用六行

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

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?) 反而读着比楼主的还费解一些，当然封装之后也关系不大。

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

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

基本全部赞同。但对最后一句话不完全认同。我认为像这个例子所在的情况可能没有重用需求，并且不会影响可读性 param[:type_id] = params[:thirdCategory].presence || params[:secondCategory].presence || params[:topCategory].presence

1. 这个是在 api 内部对参数做处理，这种 case 写法合理的情况下很少有重用的可能;
2. 看到 param[:type_id] =这里必然就会清楚后面值指代的意思是 type_id, 抽个名为 type_id 的方法对可读性不会有影响
3. 在这种情况下抽个方法，代码结构怎么组织，会不会有滥用风险等也是值得考虑的。
4. 所以我觉得一切写法还是要参考业务和需求; 任何百分百的东西都是伪命题

可读性 > 可扩展 > 优雅【这是三个不同的层面】

上面的都可以，我再写一种：

params.delete_if{|k, v| v.blank?}.values[0]

是的。

关于你说的第 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 方法——通过这种方式来隔离作用域，这样想要组织代码确实比较头疼。

关于 临时变量 bad_smell 这个确实不太理解; 如果可以的话请详细解释一下好吗; 因为就我个人而言，是比较反对把不影响可读性，又没有复用需求的代码抽成方法的

在《重构--ruby 版》一书中，作者详细描述过临时变量是代码的 bad smell，具体原因我只记得一点了，就是临时变量不能在方法之间共用（也有可能我没记住关键的原因）。作者倡导定义查询方法替换临时变量。其实我觉得严格按照作者说的做有点矫枉过正，实际上在项目中，我也不会把木有复用需求的代码抽成方法。但是谁能保证以后一定没有复用需求呢？

其实你的这段代码可读性真的很差 对于阅读方来说 总得打断去看重构方法的实现 毕竟我没法 100% 确保接口是一定正确的

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

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

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

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

#重构前
  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 的警觉。

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

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

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

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

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

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

我认同在能提升可读性和维护性的情况下用方法代替临时变量; 但软件工程没有银弹，我认为这是一个最多只有 73, 甚至可能 64 的设计。

可读性

在方法数量合理 (这个是我个人感觉) 和取名合理的情况下，我觉得大部分方法都可以增加可读性; 但是注意是取名合理的前提下，如果对这点有些感受的人都知道，程序员几乎 50% 的时间都在思考如何取名，如果只是像例子中那样简单的把临时变量名改成方法名，我觉得对可读性是毫无帮助的，如果需要自己归纳总结，好坏不说，成本可能不会比后期重用重构的成本低。

可维护性

但是谁能保证以后一定没有复用需求呢？

我觉得这句话的出发点是错的，没有可预见的复用需求的的前提下我认为这么思考是过度设计。不说有没有价值，最简单的一点，我觉得存在被滥用的可能性。

关于整理代码的意识，很多时候——就像你说的——人们习惯于总结出一些非此即彼的“百分之百”的标准，更不愿意在具体的场景下具体分析

像你说的这句话一样，人都有惰性，很多人喜欢下意识的重用之前留下的东西，无论语义是否一致，只要代码逻辑有可复用的地方就会果断的修改和重用。这种语义放大的设计对后期的修改和维护是极不友好的，但是却很容易出现。所以针对这点，我会在最开始就考虑不过度设计这些东西。临时变量就好好的当你的临时变量，不要给别人复用的可能，最开始就不给别人想法，, 其实可以预防很多的问题

我见到这个谨发一下感慨。

在之前 Ruby 初始阶段的时候 我也经常追求有些炫技式的所谓的优雅，追求代码行数短小。后来随着见得多了，才知道是多么幼稚。写代码是一个应对变化、协作的过程，不是自说自话。

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

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

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