Ruby China

重构 常见重构手法 (下)

qinfanpeng · December 29, 2016 · Last by hemengzhi88 replied at December 31, 2016 · 7862 hits

原文地址：http://qinfanpeng.github.io/jekyll/update/2016/12/18/common_refactor_skills_part_two.html

Common Reactor Skills Part Two

[slide]

Agenda

  • Replace Magic number with Constant {:&.fadeIn}
  • Decompose Conditional
  • Replace Nested Conditional with Guard Clauses
  • Consolidate Conditional Expression
  • Consolidate Duplicate Conditional Fragments
  • Remove Control Flag
  • Sparate Query from Modifer
  • Introduce Named Parameter

[slide]

Agenda

  • Replace Magic number with Constant
  • Decompose Conditional
  • Replace Nested Conditional with Guard Clauses
  • Consolidate Conditional Expression
  • Consolidate Duplicate Conditional Fragments
  • Remove Control Flag
  • Sparate Query from Modifer
  • Introduce Named Parameter

[slide]

const calculatePrice = (order) => {
    return basePrice(order) - quantityDiscount(order) + shipping(order)
}

const quantityDiscount = ({ itemPrice }) => {
    return Math.max(0, quantity - 500) * itemPrice * 0.05
}

const shipping = function (order) {
    return Math.min(100, basePrice(order) * 0.1)
}

改善空间？

  • 魔法数不可读，妨碍理解 {:&.fadeIn}
  • 魔法数容易重复，从而散落于代码各处，致使散弹式修改
  • 不具备可搜索性

[slide]

Replace Magic number with Constant(用常量替换魔法数)

const QUANTITY_DISCOUNT_THRESHOLD = 500
const QUANTITY_DISCOUNT_RATE = 500
const SHIPPING_RATE = 0.1
const MIN_SHIPPING = 100

const quantityDiscount = ({ itemPrice }) => {
    return Math.max(0, quantity - QUANTITY_DISCOUNT_THRESHOLD) * itemPrice * QUANTITY_DISCOUNT_RATE
}

var shipping = function (order) {
    return Math.min(MIN_SHIPPING, basePrice(order) * SHIPPING_RATE)
}

[slide]

思考 - 放置常量的位置？

  • 变化频率 {:&.fadeIn}
  • 使用范围

[note] 变化越频繁、使用范围越广，更应放到全局性的地方，甚至是配置文件；使用范围、变化越少，则可就近放置（比如当前文件顶部？） [/note]

[slide]

字符串字面量与魔法数的异同？

const shipping = function (order) {
    return Math.min(100, basePrice(order) * 0.1)
}

determineLegendAlignment(model, ['top', 'topCenter'])
determineLegendAlignment(model, ['left', 'leftMiddle'])
determineLegendAlignment(model, ['right', 'rightMiddle', 'rightBottom'])
determineLegendAlignment(model, ['bottom', 'bottomCenter'])

和魔法数的异同？

  • “魔法数不可读，妨碍理解”，字符串字面量无此问题 {:&.fadeIn}
  • “魔法数容易重复，从而散落于代码各处，致使散弹式修改”，有此问题，但频率不高
  • 字符串字面量容易出现拼写错误
  • 要不要抽常量？个人觉得尽量抽

[slide]

const calculateCharge = (date, quantity) => {
  if (SUMMER_START <= date && date <= SUMMER_END) {
    return quantity * SUMMER_RATE
  } else {
    return quantity * WINTER_RATE + WINTER_SERVICE_CHARGE 
  }
}

改善空间？

  • 未能突出领域概念 {:&.fadeIn}
  • 未能突出各个分支

[slide]

Decompose Conditional(分解条件表达式)

const calculateCharge = (date, quantity) => {
  if (SUMMER_START <= date && date <= SUMMER_END) {
    return quantity * SUMMER_RATE
  } else {
    return quantity * WINTER_RATE + WINTER_SERVICE_CHARGE 
  }
}

const calculateCharge = (date, quantity) => {
  return isInSummer(date) ? summerCharge(date) : winterCharge(date)
}

const isInSummer = (date) => SUMMER_START <= date && date <= SUMMER_END

const summerCharge = (quantity) => quantity * SUMMER_RATE

const winterCharge = (quantity) => quantity * WINTER_RATE + WINTER_SERVICE_CHARGE

[slide]

[magic data-transition="cover-circle"]

最容易从以下代码结构联想到什么？

const calculatePayAmount = () => {
  let result = 0.0

  if (isDead()) {
    result = deadAmount()
  } else {
    if (isSeparated()) {
      result = separatedAmount()
    } else {
      if (isRetired()) {
        result = retiredAmount()
      } else {
        result = normalPayAmount()
      }
    }
  }

  return result
}

====

迷宫

鹿角

改善空间？

  • 条件嵌套，致使阅读负担陡增，不容易找出主要逻辑
  • 摆脱单一出口原则的束缚
  • 乱用 if-else 结构

[/magic]

[slide]

if-else 结构意味着什么？

  • 两个分支都是正常情况 {:&.zoomIn}
  • 两个分支重要程度相当，该受到相同的重视程度
  • 两个分支发生的概率相当
  • 两个分支确实属于非此即彼的关系

[slide]

Guard Clauses（卫语句）

const calculatePayAmount = () => {
  if (isDead()) return deadAmount()
  if (isSeparated()) return separatedAmount()

  // ...
  // ...
  // ...

卫语句意味着什么？

  • 这种情况很罕见，如果它真地发生了，稍作处理，退出即可 {:&.zoomIn}

[slide]

何时使才用 if-else？

  • 两个分支都是正常情况 {:&.zoomIn}
  • 两个分支重要程度相当，该受到相同的重视程度
  • 两个分支发生的概率相当
  • 两个分支确实属于非此即彼的关系
  • 两个分支代码量相当

[slide]

[magic data-transition="cover-circle"]

影响选用 if-else/卫语句的因数

-- 主要观察点：两个分支是否都是正常情况

const computeDisabilityAmount = () => {
  if (isNotEligibleForDisability()) {
    return 0.0
  } else {
    // compute disability amount
    // ...
    // ...
  }
}

const isNotEligibleForDisability = () => {
  return seniority < 0 || monthsDisabled > 12 || isPartTime()
}

====

const computeDisabilityAmount = () => {
  if (isNotEligibleForDisability()) return 0.0

  // compute disability amount
  // compute disability amount
  // ...
  // ...
}

const isNotEligibleForDisability = () => {
  return seniority < 0 || monthsDisabled > 12 || isPartTime()
}

[/magic]

[slide]

[magic data-transition="cover-circle"]

影响选用 if-else/卫语句的因数

-- 主要观察点：两个分支代码量是否相当

handleCellWithTooltip(fieldValue, field) {
    if (this.props.needTooltipFieldNames.includes(field)) {
      const tooltipId = uniqueId('fieldValue_')
      return (
        <span key={tooltipId}>
          <div data-tip data-for={tooltipId}>{fieldValue}</div>
          <Tooltip id={tooltipId} place='right'>
            <div>{fieldValue}</div>
          </Tooltip>
        </span>
      )
    } else {
      return <span key={uniqueId('cell_')}>{fieldValue}</span>
    }
  }

====

handleCellWithTooltip(fieldValue, field) {
    if (!this.props.needTooltipFieldNames.includes(field)) return <span key={uniqueId('cell_')}>{fieldValue}</span>

   const tooltipId = uniqueId('fieldValue_')
   return (
     <span key={tooltipId}>
       <div data-tip data-for={tooltipId}>{fieldValue}</div>
       <Tooltip id={tooltipId} place='right'>
         <div>{fieldValue}</div>
       </Tooltip>
     </span>
   )
}

[/magic]

[slide]

Replace Nested Conditional with Guard Clauses (用卫语句替换嵌套条件表达式)


const calculatePayAmount = () => {
  let result = 0.0

  if (isDead()) {
    result = deadAmount()
  } else {
    if (isSeparated()) {
      result = separatedAmount()
    } else {
      if (isRetired()) {
        result = retiredAmount()
      } else {
        result = normalPayAmount()
      }
    }
  }

  return result
}




const calculatePayAmount = () => {
  if (isDead()) return deadAmount()
  if (isSeparated()) return separatedAmount()
  if (isRetired()) return retiredAmount()
  
  return normalPayAmount()
}

[slide]

[magic data-transition="cover-circle"]

消除条件嵌套

const getAdjustedCapital = () => {
  const result = 0

  if (capital > 0) {
    if (intRate > 0 && duration > 0) {
      result = (_income / _duration) * ADJ_FACTOR
    }
  }

  return result
}

====

条件取反 -- 识别异常情况

const getAdjustedCapital = () => {
  if (capital <= 0) return 0
  if (!(intRate > 0 && duration > 0)) return 0

  return (_income / _duration) * ADJ_FACTOR
}

====

简化取反后的条件

const getAdjustedCapital = () => {
  if (capital <= 0) return 0
  if (intRate <= 0 || duration <= 0)) return 0

  return (_income / _duration) * ADJ_FACTOR
}

[/magic]

[slide]

使用 if-else 的注意事项

  • 尽量避免条件表达式嵌套 {:&.zoomIn}
  • 个人建议别优先使用 if-else

[slide]

const computeDisabilityAmount = () => {
  if (seniority < 0) return 0
  if (monthsDisabled > 12) return 0
  if (isPartTime()) return 0

  // compute disability amount
}

改善空间？

  • 喧宾夺主，函数很大一部分空间被验证逻辑霸占 {:&.fadeIn}
  • 可以合并的条件检测

[slide]

Consolidate Conditional Expression(合并条件表达式)

const computeDisabilityAmount = () => {
  if (seniority < 0) return 0
  if (monthsDisabled > 12) return 0
  if (isPartTime()) return 0

  // compute disability amount
}
  • {:&.fadeIn}
const computeDisabilityAmount = () => {
  if (seniority < 0 || monthsDisabled > 12 || isPartTime()) return 0
  // compute disability amount
}

const computeDisabilityAmount = () => {
  isNotEligibleForDisability() return 0
  // compute disability amount
}

const isNotEligibleForDisability = () => {
  return seniority < 0 || monthsDisabled > 12 || isPartTime()
}

[slide]

if (isSpecialDeal()) {
  totalPrice = price * SPECIAL_DEAL_DISCOUNT_RATE
  send()
} else {
  totalPrice = price * NORMAL_DISCOUNT_RATE
  send()
}

改善空间？

  • 并没严格把变和不变区分开 {:&.fadeIn}

[slide]

Consolidate Duplicate Conditional Fragments (合并重复条件代码片段)

if (isSpecialDeal()) {
  totalPrice = price * SPECIAL_DEAL_DISCOUNT_RATE
  send()
} else {
  totalPrice = price * NORMAL_DISCOUNT_RATE
  send()
}

if (isSpecialDeal()) {
  totalPrice = price * SPECIAL_DEAL_DISCOUNT_RATE
} else {
  totalPrice = price * NORMAL_DISCOUNT_RATE
}
send()

[slide]

Demo of Consolidate Duplicate Conditional Fragments

handleMenuOnChange(selectedItem) {
  if (this.props.length > 1) {
    this.setState({
      selectedMeasure:{
        fieldId: selectedItem.value
      }
    })
  }
  else {
    this.setState({
      selectedMeasure: selectedItem.measure
    })
  }
}
  • {:&.fadeIn}
handleMenuOnChange(selectedItem) {
  const selectedMeasure = this.props.measures.length > 1 ? {fieldId: selectedItem.value} : selectedItem.measure 
  this.setState({ selectedMeasure })
}

[slide]

const checkSecurity = (people) => {
  const miscreant = findMiscreant(people)
  // doSomethingElse(miscreant)
}

const findMiscreant = (people) => {
  let found = false
  people.forEach(person => {
    if (!found) {
      if (person === 'Bob') {
        sendAlert()
        found = true
      }
      if (person === 'Jack') {
        sendAlert()
        found = true
      }
    }
  })
}

改善空间？

  • 丑陋的控制位（found） {:&.fadeIn}
  • 查询函数中暗含副作用，且无法从函数名中看出
  • 臃肿的 for 循环

[slide]

[magic data-transition="cover-circle"]

Remove Control Flag (移除控制位)

-- 移除前

const findMiscreant = (people) => {
  let found = false
  people.forEach(person => {
    if (!found) {
      if (person === 'Bob') {
        sendAlert()
        found = true
      }
      if (person === 'Jack') {
        sendAlert()
        found = true
      }
    }
  })
}

====

Remove Control Flag (移除控制位)

-- 移除后

const findMiscreant = (people) => {
  people.forEach(person => {
      if (person === 'Bob') {
        sendAlert()
        break;
      }
      if (person === 'Jack') {
        sendAlert()
        break;
      }
    }
  })
}

[/magic]

[slide]

[magic data-transition="cover-circle"]

Sparate Query from Modifer (将查询函数和修改函数分开)

-- 分离前

const findMiscreant = (people) => {
  people.forEach(person => {
      if (person === 'Bob') {
        sendAlert()
        break;
      }
      if (person === 'Jack') {
        sendAlert()
        break;
      }
    }
  })
}

====

Sparate Query from Modifer (将查询函数和修改函数分开)

-- 分离后

const checkSecurity = (people) => {
  const miscreant = findMiscreant(people)
  if (!isUndefined(miscreant)) sendAlert(miscreant)
  // doSomethingElse(miscreant)
}

const findMiscreant = (people) => {
  const miscreants = ['Bob', 'Jack']
  return find(people, person => miscreants.includes(person))
}

====

Sparate Query from Modifer (将查询函数和修改函数分开)

-- 万不得已

const findMiscreantAndSendAlert = (people) => {
  people.forEach(person => {
    if (!found) {
      if (person === 'Bob') {
        sendAlert(person)
        return person
      }
      if (person === 'Jack') {
        sendAlert(person)
        return person
      }
    }
  })
}

[/magic]

[slide]

const SearchCriteria = buildSearchCriteria(5, "0201485672")
  • {:&.fadeIn}
const SearchCriteria = buildSearchCriteria(5, "0201485672", false)

buildSearchCriteria(5, "0201485672", false, 'JavaScript : The Good Parts')

const buildSearchCriteria = (authorId, ISBN, includeSoldOut, title) => {
  // ...
}

改善空间？

  • 实参为数字、布尔值时不可读 {:&.fadeIn}
  • 参数较多时，逻辑混乱，不易理解，且容易弄错参数顺序

[slide]

[magic data-transition="cover-circle"]

Introduce Named Parameter (引入具名参数)

const buildSearchCriteria = ({ authorId, ISBN, includeSoldOut, title }) => {
  // ...
}

const SearchCriteria = buildSearchCriteria({
  authorId: 5, 
  ISBN: "0201485672", 
  includeSoldOut: false, 
  title: 'JavaScript : The Good Parts'
})

====

至少提取解释变量

const **authorId** = 5
const ISBN = '0201485672'
const includeSoldOut = false
const title = 'JavaScript The Good Parts'

const SearchCriteria = buildSearchCriteria(authorId, ISBN, includeSoldOut, title)

[/magic]

[slide]

参考资料

  1. 《重构》
  2. 《编写可读的艺术》
  3. 《代码整洁之道》

[slide]

Thank You & QA

10 likes
lithium4010 #1 December 30, 2016

有本书叫《编写可阅读代码的艺术》，内容很类似

qinfanpeng #2 December 30, 2016

#1 楼 @lithium4010 你说的这本书也不错。不过这里的内容主要根据《重构》里来行文的

1 likes
plpl123456 #3 December 30, 2016

江湖朋友赞一个👍

hemengzhi88 #5 December 31, 2016

#3 楼 @plpl123456 此地无银三百块！

tesla_lee in 这种代码有什么好的重构方法吗？ mention this topic. 12 Jan 18:36
You need to Sign in before reply, if you don't have an account, please Sign up first.