@ledsun blog

Hのキーがhellで、Sのキーがslaveだ、と彼は思った。そしてYのキーがyouだ。

あるリファクタリング

次のような関数がありました。 どうしたもんかなあ?と悩みました。

import validateAnnotation from './validateAnnotation'
import convertBeginAndEndToInteger from './convertBeginAndEndToInteger'

export default function (
  spanContainer,
  entityContainer,
  attributeContainer,
  relationContainer,
  text,
  spans,
  rowData,
  trackNumber = ''
) {
  const { accept, reject } = validateAnnotation(text, spans, rowData)

  const { typeSetting } = accept
  const typesettings = typeSetting.map((src) => ({
    ...src,
    span: convertBeginAndEndToInteger(src.span)
  }))
  spanContainer.addSource(typesettings, 'typesetting')

  const { denotation } = accept
  const denotations = denotation.map((src) => ({
    ...src,
    id: src.id ? trackNumber + src.id : null,
    span: convertBeginAndEndToInteger(src.span)
  }))
  spanContainer.addSource(denotations, 'denotation')
  entityContainer.addSource(denotations, 'denotation')

  const { block } = accept
  const blocks = block.map((src) => ({
    ...src,
    id: src.id ? trackNumber + src.id : null,
    span: convertBeginAndEndToInteger(src.span)
  }))
  spanContainer.addSource(blocks, 'block')
  entityContainer.addSource(blocks, 'block')

  const { relation } = accept
  const relations = relation.map((src) => ({
    ...src,
    id: src.id ? trackNumber + src.id : null,
    subj: trackNumber + src.subj,
    obj: trackNumber + src.obj
  }))
  relationContainer.addSource(relations)

  const { attribute } = accept
  const attributes = attribute.map((src) => ({
    ...src,
    id: src.id ? trackNumber + src.id : null,
    subj: trackNumber + src.subj,
    obj: src.obj
  }))
  attributeContainer.addSource(attributes)

  return reject
}

2時間くらいいじくり回して、こうなったっす。

import IDConflictResolver from './IDConflictResolver'
import convertBeginAndEndOfSpanToInteger from './convertBeginAndEndOfSpanToInteger'

export default function (
  spanContainer,
  entityContainer,
  attributeContainer,
  relationContainer,
  accept,
  trackNumber = ''
) {
  const [typeSettings, denotation, block] = convertBeginAndEndOfSpanToInteger(
    accept.typeSetting,
    accept.denotation,
    accept.block
  )
  const { relation, attribute } = accept
  const { denotations, blocks, relations, attributes } = new IDConflictResolver(
    trackNumber
  ).addTrackNumberAsIDPrefix(denotation, block, relation, attribute)

  spanContainer.addSource(typeSettings, 'typesetting')
  spanContainer.addSource(denotations, 'denotation')
  spanContainer.addSource(blocks, 'block')
  entityContainer.addSource(denotations, 'denotation')
  entityContainer.addSource(blocks, 'block')
  relationContainer.addSource(relations)
  attributeContainer.addSource(attributes)
}

当初「データ取り込みの前処理部分なので、綺麗に書きようがないのかなあ?」と思いましたが、やればそれなりに読める形になりました。 やはり、単に脳みそがサボりたくて、賢そうなことを言っていただけのようです。

あとから見るとリファクタリングのコンセプトは、データ種別単位で

  1. 前処理1
  2. 前処理2
  3. 取り込み

とやっていたのを、処理単位にまとめ直したのでした。 それで処理に意図がわかる名前 IDConflictResolver がつけられるようになったのが良かったです。

convertBeginAndEndOfSpanToInteger も、もっとHowでなくWhatを表す名前にしても良いかもしれません。