重构常见重构手法 (下)

qinfanpeng · 2016年12月29日 · 最后由 hemengzhi88 回复于 2016年12月31日 · 7872 次阅读

原文地址：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]

参考资料

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

[slide]

Thank You & QA

10 个赞

lithium4010 #1 2016年12月30日

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

qinfanpeng #2 2016年12月30日

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

1 个赞

plpl123456 #3 2016年12月30日

江湖朋友赞一个

qinfanpeng #4 2016年12月30日

#3 楼 @plpl123456

hemengzhi88 #5 2016年12月31日

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

tesla_lee 在这种代码有什么好的重构方法吗？提及了此话题。 01月12日 18:36

需要登录后方可回复, 如果你还没有账号请注册新账号

10 个赞

共收到 5 条回复

收到新回复，点击立即加载

重构 常见重构手法 (下)

重构 常见重构手法 (下)

Common Reactor Skills Part Two

Agenda

Agenda

改善空间？

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

思考 - 放置常量的位置？

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

和魔法数的异同？

改善空间？

Decompose Conditional(分解条件表达式)

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

改善空间？

if-else 结构意味着什么？

Guard Clauses（卫语句）

卫语句意味着什么？

何时使才用 if-else？

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

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

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

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

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

消除条件嵌套

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

简化取反后的条件

使用 if-else 的注意事项

改善空间？

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

改善空间？

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

Demo of Consolidate Duplicate Conditional Fragments

改善空间？

Remove Control Flag (移除控制位)

-- 移除前

Remove Control Flag (移除控制位)

-- 移除后

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

-- 分离前

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

-- 分离后

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

-- 万不得已

改善空间？

Introduce Named Parameter (引入具名参数)

至少提取解释变量

参考资料

Thank You & QA

重构常见重构手法 (下)

重构常见重构手法 (下)