Skip to content

Instantly share code, notes, and snippets.

@yamatatsu
Last active August 2, 2018 06:08
Show Gist options
  • Save yamatatsu/bc01ce3033951b40f97aac4d8cb40f07 to your computer and use it in GitHub Desktop.
Save yamatatsu/bc01ce3033951b40f97aac4d8cb40f07 to your computer and use it in GitHub Desktop.

前提

こんな話がある わかりやすいコメント <<<<<<{超えられない壁}<<<<<< 読みやすいコード <<<<<<{超えられない壁}<<<<<< わかりやすい命名

そんな話を書きたい

やってみよう

わかりやすいコメント && 読みやすい(?)コード

export default class NabeatsuService {
  fizzBazz(from, to) {
    // 読み上げる数字の配列を作成
    const arr = []
    for (let i = from; i <= to; i++) {
      arr.push(i)
    }

    // 読み上げる
    arr.forEach(i => {
      // aho && dog
      if (i % 3 === 0 && i % 5 === 0) {
        console.log('aho wan')
      }
      // aho
      else if (i % 3 === 0) {
        console.log('aho')
      }
      // dog
      else if (i % 5 === 0) {
        console.log('wan')
      } else {
        console.log(i)
      }
    })
  }
}

わかりやすい命名

publicに公開するメソッド(fizzBazz)には基本的に実装を書かない

export default class NabeatsuService {
  // ↓外から呼ばれる関数は簡潔に。読めば分かるコードにする。
  fizzBazz(from, to) {
    this.range(from, to).forEach(i => {
      console.log(this.getWord(i))
    })
  }

  range(from, to) {
    // return [...Array(to - from + 1).keys()].map(i => i + from) でもいい
    const arr = []
    for (let i = from; i <= to; i++) {
      arr.push(i)
    }
    return arr
  }

  getWord(i) {
    if (this.isAho(i) && this.isDog(i)) {
      return 'aho wan'
    } else if (this.isAho(i)) {
      return 'aho'
    } else if (this.isDog(i)) {
      return 'wan'
    } else {
      return i
    }
  }

  isAho(i) {
    return i % 3 === 0
  }
  isDog(i) {
    return i % 5 === 0
  }
}

ところでjsってさ

privateメソッドないじゃん? publicに公開するメソッドがどれなのかわからんやん?

NabeatsuServiceの調子でメソッド生えまくって、でも呼ばれるのfizzBazz()だけ。。。。 そのコードメンテしたくないじゃん?

結果private切りたくなくなってfat methodが生まれるやん?

なのでこうだ

classはpublicなIFのみを提供する 今回はServiceなので「それ関数でいいじゃん」ってなるかもだけど、ドメインオブジェクトでも同じこと。

// 外から呼ばれるモノだけ公開
export default class NabeatsuService {
  fizzBazz(from, to) {
    range(from, to).forEach(i => {
      console.log(getWord(i))
    })
  }
}

// 他はファイル内に閉じてる
const range = (from, to) => {
  const arr = []
  for (let i = from; i <= to; i++) {
    arr.push(i)
  }
  return arr
}

const getWord = (i) => {
  if (isAho(i) && isDog(i)) {
    return 'aho wan'
  } else if (isAho(i)) {
    return 'aho'
  } else if (isDog(i)) {
    return 'wan'
  } else {
    return i
  }
}

const isAho = (i) => i % 3 === 0
const isDog = (i) => i % 5 === 0

余談

fizzBazz の主たる副作用である console.log ってprivateなメソッドの中に隠蔽して say みたいなメソッドにできるんだけど、 こういう書き方はあんまりよくない。

fizzBazz(from, to) {
  range(from, to).forEach(say)
}

これはCQSっていうCQRSの元になってコーディング原則がある。 Command/Query Separation 情報の取得を行う関数と、副作用を起こす関数は分離すべき、という考え方である。 range, getWord, isAho, isDog はすべてQueryになっている。 なぜこれが良いかというと、

  • Queryはテストしやすい
  • Queryは合成可能
    • Queryの中でQueryの100万回使ってもQueryがCommandになることはない
    • 逆に一箇所でもCommandを使えば、その関数はCommandとなる
  • 重大な不可逆バグはCommandで起こる

という話がある。

でも、テストしやすさに関してはprivateにしてしまうとテスト出来ないからなー!!! やっぱ「index.jsのみをpublicとする」みたいな紳士協定に頼る他無いのか。。。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment