重構:改善既有程式的設計 — 簡化條件邏輯

這次閱讀簡化條件邏輯。

Decompose Conditional 分解條件邏輯

這邊這點在書中講的是把條件式抽成 function 像是這樣

if(a !== 10 && 'b' in object) {
  // ...
} else {
  // ...
}

改成

if(){
  // ...
} else {
  // ...
}
 
function checkCondition () {
  return a !== 10 && 'b' in object
}

但我個人其實不喜歡這種做法,原因有 2

  1. function naming 要為何? 應該要是 get 開頭還是 用 is/has 之類的開頭? 這個 naming,雖然這很看團隊,但我個人的要求是 is/has 開頭習慣給 boolean 變數使用,get 開頭也可以但我個人更習慣 get 給取得一個資料所用,因此我更習慣的是使用 check,所以這又回到了習慣問題,但是習慣問題我覺得是最難推動的
  2. 我要確認這個 condition 是不是得跳來跳去做確認,是不是增加了 code review 的困難度

因此我的做法通常會是

const isCondition = a !== 10 && 'b' in object
 
if(isCondition){
  // ...
} else {
  // ...
}

這樣通常變數會在 if-else 前做宣告,因此可以直接看到,問題就回到了 boolean 值的 naming, 這是一個更好溝通的習慣

Consolidate Conditional Expression 合併條件式

if (a) return 0
if (b) return 0
if (c) return 0
 
 
// to
const isZero = a || b || c
if(isZero) return 0

這個我也很常用,把相同的執行內容合併起來,但有時候有個小小隱憂,萬一不是平行的 condition 並且有先後順序時就要小心使用,稍微 dry run 一下確保邏輯上是正確的(testing cover 是一個好方式)

Replace Nested Conditional with Guard Clauses 內嵌條件式換成防衛述句

function getPayAmount(){
  let result;
  if(isDead) {
    result = deadAmount();
  }else{
    if(isSeparated){
      result = separatedAmount()
    } else {
      if(isRetired) {
        result = retiredAmount()
      } else {
        result = normalPayAmount()
      }
    }
  }
  
  return result;
}

我看完書中範例覺得就是把一連串的條件式抽成 function 然後用 early return 處理

function getPayAmount(){
  if(isDead) return deadAmount()
  if(isSeparated) return separatedAmount()
  if(isRetired) return retiredAmount()
  
  return normalPayAmount()
}

Replace Conditional with Polymorphism 條件式換成多型

情況一:switch 在分派「型別」

書中的鳥類範例。同一個 switch 依 type 分派,而每個 case 其實是在講「這種鳥怎麼算」:

function plumages(birds) {
  return new Map(birds.map(b => [b.name, plumage(b)]))
}
 
function plumage(bird) {
  switch (bird.type) {
    case 'EuropeanSwallow':
      return 'average'
    case 'AfricanSwallow':
      return bird.numberOfCoconuts > 2 ? 'tired' : 'average'
    case 'NorwegianBlueParrot':
      return bird.voltage > 100 ? 'scorched' : 'beautiful'
    default:
      return 'unknown'
  }
}

改成每個型別各自一個 class,把 case 的內容搬進去:

class Bird {
  constructor(data) {
    Object.assign(this, data)
  }
  get plumage() {
    return 'unknown'
  }
}
 
class EuropeanSwallow extends Bird {
  get plumage() {
    return 'average'
  }
}
 
class AfricanSwallow extends Bird {
  get plumage() {
    return this.numberOfCoconuts > 2 ? 'tired' : 'average'
  }
}
 
class NorwegianBlueParrot extends Bird {
  get plumage() {
    return this.voltage > 100 ? 'scorched' : 'beautiful'
  }
}
 
function createBird(data) {
  switch (data.type) {
    case 'EuropeanSwallow':    return new EuropeanSwallow(data)
    case 'AfricanSwallow':     return new AfricanSwallow(data)
    case 'NorwegianBlueParrot':return new NorwegianBlueParrot(data)
    default:                   return new Bird(data)
  }
}
 
function plumages(birds) {
  return new Map(birds
    .map(b => createBird(b))
    .map(bird => [bird.name, bird.plumage]))
}

注意 switch 沒有消失,它搬到 factory 去了——差別在整個系統只剩「一個」switch,而不是每多一個行為就多抄一份同樣的 switch。

情況二:基本情況 + 少數變體

另一種用法不是分派型別,是「大部分走通則,某些情況例外」。原本長這樣:

function rating(voyage, history) {
  const vpf = voyageProfitFactor(voyage, history)
  const vr  = voyageRisk(voyage)
  const chr = captainHistoryRisk(voyage, history)
  if (vpf * 3 > (vr + chr * 2)) return 'A'
  return 'B'
}
 
function captainHistoryRisk(voyage, history) {
  let result = 1
  if (history.length < 5) result += 4
  // 中國航線的老手風險要再扣一點
  if (voyage.zone === 'china' && hasChina(history)) result -= 2
  return Math.max(result, 0)
}
 
function voyageProfitFactor(voyage, history) {
  let result = 2
  if (voyage.zone === 'china') result += 1
  // 中國航線 + 有經驗 → 另一套加成
  if (voyage.zone === 'china' && hasChina(history)) {
    result += 3
    if (history.length > 10) result += 1
  }
  return result
}

把「通則」留在基底類別,「例外」變成一個子類別,只覆寫它真正不一樣的那幾個方法:

class Rating {
  constructor(voyage, history) {
    this.voyage = voyage
    this.history = history
  }
 
  get value() {
    const vpf = this.voyageProfitFactor
    const vr  = this.voyageRisk
    const chr = this.captainHistoryRisk
    return (vpf * 3 > (vr + chr * 2)) ? 'A' : 'B'
  }
 
  get captainHistoryRisk() {
    const result = this.history.length < 5 ? 5 : 1
    return Math.max(result, 0)
  }
 
  get voyageProfitFactor() {
    return this.voyage.zone === 'china' ? 3 : 2
  }
}
 
class ExperiencedChinaRating extends Rating {
  get captainHistoryRisk() {
    return Math.max(super.captainHistoryRisk - 2, 0)
  }
 
  get voyageProfitFactor() {
    return super.voyageProfitFactor + 3 + (this.history.length > 10 ? 1 : 0)
  }
}
 
function createRating(voyage, history) {
  return (voyage.zone === 'china' && hasChina(history))
    ? new ExperiencedChinaRating(voyage, history)
    : new Rating(voyage, history)
}
 
function rating(voyage, history) {
  return createRating(voyage, history).value
}

重點是 voyage.zone === 'china' && hasChina(history) 這個判斷從三個地方收斂成一個(factory),其他地方只要問「我是哪一種 Rating」。

以 case 1 來說,我更喜歡用 map 的方式(比較是個人傾向,不喜歡 switch case)

function createBird(data) {
  const birdMap = {
    EuropeanSwallow,
    AfricanSwallow,
    NorwegianBlueParrot,
  }
 
  const BirdClass = birdMap[data.type] ?? Bird
 
  return new BirdClass(data)
}
 

我個人認為這樣在閱讀上其實更容易閱讀,但比較偏向個人習慣, switch case 搭配多型我也覺得不錯,唯一的小缺點應該是 class 都有的問題,有時會覺得各個要繼承導致 code 有點長

Introduce Special Case 引入特例(Null Object)

適用情境:同一個「特殊值」在很多地方被重複檢查,而每個檢查之後做的事都一樣。書中的例子是 customer 可能是字串 'unknown':

const aCustomer = site.customer
 
// 散落在各處的同一種檢查
let customerName
if (aCustomer === 'unknown') customerName = 'occupant'
else customerName = aCustomer.name
 
let plan
if (aCustomer === 'unknown') plan = registry.billingPlans.basic
else plan = aCustomer.billingPlan
 
let weeksDelinquent
if (aCustomer === 'unknown') weeksDelinquent = 0
else weeksDelinquent = aCustomer.paymentHistory.weeksDelinquentInLastYear

做法是替這個特例做一個物件,讓它長得跟正常物件一樣,把「unknown 的時候該回什麼」寫進去一次:

class Customer {
  get name()           { /* ... */ }
  get billingPlan()    { /* ... */ }
  set billingPlan(arg) { /* ... */ }
  get paymentHistory() { /* ... */ }
  get isUnknown()      { return false }
}
 
class UnknownCustomer {
  get isUnknown()      { return true }
  get name()           { return 'occupant' }
  get billingPlan()    { return registry.billingPlans.basic }
  set billingPlan(arg) { /* 忽略 */ }
  get paymentHistory() { return new NullPaymentHistory() }
}
 
class NullPaymentHistory {
  get weeksDelinquentInLastYear() { return 0 }
}

來源端負責把特殊值換成特例物件,呼叫端一個 if 都不用寫:

class Site {
  get customer() {
    return (this._customer === 'unknown')
      ? new UnknownCustomer()
      : this._customer
  }
}
 
// 呼叫端
const aCustomer = site.customer
const customerName    = aCustomer.name
const plan            = aCustomer.billingPlan
const weeksDelinquent = aCustomer.paymentHistory.weeksDelinquentInLastYear

不想開 class 的版本

特例物件不一定要是 class。資料是純資料(record)時,回一個形狀一樣的物件字面值就夠了:

function createUnknownCustomer() {
  return {
    isUnknown: true,
    name: 'occupant',
    billingPlan: registry.billingPlans.basic,
    paymentHistory: {
      weeksDelinquentInLastYear: 0,
    },
  }
}
 
function getCustomer(site) {
  return site.customer === 'unknown'
    ? createUnknownCustomer()
    : site.customer
}

差別只在:class 版可以 lazy(getter 被呼叫才算),字面值版是當場把所有欄位算好。

真的需要判斷「是不是特例」時

保留一個 isUnknown,而不是讓呼叫端回去比對 === 'unknown':

// before
if (aCustomer === 'unknown') { /* ... */ }
 
// after
if (aCustomer.isUnknown) { /* ... */ }

這樣特殊值長什麼樣只有一個地方知道(factory),之後要把 'unknown' 換成 null 或換成某個 id,呼叫端全都不用動。

這個在大部分的實際情況我也是類似作法

我認為改善判斷式的寫法可以很好的改善閱讀性,只是做一點邏輯上的整理,可以把繁雜的判斷式寫得更乾淨,再透過一些小技巧,整個程式會非常一目瞭然,我以前甚至是一點點 nested 的 判斷都不能接受,例如

if (a) {
	if(b) {
		return 1
	}else {
	  return 2
	}
} else {
	return c
}

我會想寫成

if(a && b) return 1
if(a && !b) return 2
return c

但現在只會對我自己寫的 code 有這種要求,但是 review 時只要不要太誇張我通常都會接受


抽出的原子筆記

未抽成筆記:Guard Clauses 那節(內容等同書上原文,已併入上面第三篇當對照)。