This post is Private. Only a writer or those who know its URL can access this post.

動いているものは触らない!

More than 1 year has passed since last update.

自己紹介

  • 名前: 政倉 智 (まさくら とも)
  • 所属: codeArts 株式会社
  • 趣味:
    • バイク
    • プログラミング
    • モータースポーツ
  • お仕事ください

みなさん、言われてますよね?


「動いているものは触るな!」


わりと無視しますけどね!


先輩「このバグ直しておいて」

後輩「分かりました!」

後輩「原因分かったぞ!」

後輩「直しました!」

先輩「おー、確認してみる」


先輩「直っとらんやんけ!」

後輩「動いているものは触るなと言われたので、仕様にしました!」


動いているものは触らない

  • 本番に勝るテストはない (本番のためのテストだし...)
  • 触るということはテスト実績をゼロにするということ

バグ修正にかかるコストは、実装時に修正するのと比べてリリース後は 1,000 倍かかるとも言われている

http://www.atmarkit.co.jp/ait/articles/0901/26/news111_3.html


でも、触らないとバグは直らないよね?


どうやって触るかがミソ


あなたならどうします?

  • 某社の給与管理システム
  • 平社員で残業時間が 5 時間のときだけ、額面が半額になるバグが
// 模擬コード
給与額計算(社員種別, 残業時間) {
  // 1000 行くらいあるコード...
}

バグ修正をすると予測されること

  • 代わりに全社員の給料が倍になる

他の条件で正しく動作することを証明するのは困難


後ろ向きな解決方法を紹介


メソッドをリネーム (動作しなくなる)

給与額計算2(社員種別, 残業時間) {
  // 1000 行くらいあるコード...
}

動作するようにする

給与額計算(社員種別, 就業時間, 残業時間) {
  return 給与額計算2(社員種別, 残業時間)
}

給与額計算2(社員種別, 就業時間, 残業時間) {
  // 1000 行くらいあるコード...
}

バグ修正!

給与額計算(社員種別, 就業時間, 残業時間) {
  給与 = 給与額計算2(社員種別, 就業時間, 残業時間)

  if (社員種別 == 一般 && 残業時間 == 5.0) {
    給与 = 給与 * 2
  }

  return 給与
}

給与額計算2(社員種別, 就業時間, 残業時間) {
  // 1000 行くらいあるコード...
}

  • ダメっぽい方法に見える! 確かによくない!
  • んでも、バグの部分を修正するよりは安全

経験則でみんな持ってるんじゃないかな?

  • 一行の修正でも、そのメソッド全体が書き換えられたとして考える
  • 新しいメソッドの追加は、コードの修正や削除よりも安全

逆に考えるんだ!


  • なるべくコード修正が少なくなるコードを書く
  • 修正はなるべくメソッドの追加で

switch (社員種別) {
  case 一般:
    // 100 行くらい

  case 契約:
    // 50 行くらい

  case 役員:
    // 100 行くらい
}

上より下の方が安全

switch (社員種別) {
  case 一般: return 一般給与額計算(就業時間, 残業時間)
  case 契約: return 契約給与額計算(就業時間, 残業時間)
  case 役員: return 役員給与額計算(就業時間, 残業時間)
}

一般給与金額(就業時間, 残業時間) {
  // 100 行くらい
}

契約給与金額(就業時間, 残業時間) {
  // 50 行くらい
}

上よりも下のほうが安全

一般給与金額(就業時間, 残業時間) {
  給与 = 基本給()
  給与 += 就業給与金額(就業時間)
  給与 += 残業給与金額(残業時間)
  給与 += 一般社員手当()
  return 給与
}

契約給与金額(就業時間, 残業時間) {
  給与 = 基本給()
  給与 += 就業給与金額(就業時間)
  給与 += 残業給与金額(残業時間)
  return 給与
}

これを考えながら書く


// 一般社員の計算だけとりあえず書く

給与額計算(社員種別, 就業時間, 残業時間) {
  // 一般社員の給与額計算
}

// メソッドを抽出する

給与額計算(社員種別, 就業時間, 残業時間) {
  return 一般給与金額(就業時間, 残業時間)
}

一般給与金額(就業時間, 残業時間) {
  // 一般社員の給与額計算
}

// 契約社員のメソッドを作る

給与額計算(社員種別, 就業時間, 残業時間) {
  switch (社員種別) {
    case: return 一般給与金額(就業時間, 残業時間)
    case: return 契約給与金額(就業時間, 残業時間)
  }
}

一般給与金額(就業時間, 残業時間) {
  // 一般社員の給与額計算
}

契約給与金額(就業時間, 残業時間) {
  return 0 // とりあえず動くようにするだけ
}

// 契約社員のメソッドを作る

給与額計算(社員種別, 就業時間, 残業時間) {
  switch (社員種別) {
    case: return 一般給与金額(就業時間, 残業時間)
    case: return 契約給与金額(就業時間, 残業時間)
  }
}

一般給与金額(就業時間, 残業時間) {
  // 一般社員の給与額計算
}

契約給与金額(就業時間, 残業時間) {
  // 契約社員の給与額計算
}

  • ちょっと書き足す
  • ある程度できたらメソッドの抽出
  • 繰り返す

自分も、うまくコードを書けてないときは、これをやってない!


メソッドの抽出がやりにくいコードは死んじゃう


やりにくいコードの例

  • 一時変数の使いまわし
  • 一時変数の寿命が長い
  • グローバル変数の多様

あんまり悪い例ではないけど...

給与 = 0

給与 += 基本給

switch (社員種別) {
  case 一般:
    給与 += 時給 * 就業時間
    給与 += 時給 * 1.5 * 残業時間
    break;

  case: 役員:
    給与 = 役員報酬
    break;

  // ...
}

給与 += 手当

メソッドの抽出を少しずつ続ければいつかよくなる


テスト実績が極力減らないような修正を繰り返す


コピペコードは実績ポイントが激減するんで早めの対処を!


おまけ


長いメソッドのバグ修正の手段はこういうのも

長いメソッド() {
  // 100 行くらい
}

こんな感じにバグ周辺を抽出するのおすすめ

長いメソッド {
  // 50 行くらい

  一部()

  // 50 行くらい
}

一部() {
  // バグのあるあたり
}

長いメソッド {
  // 50 行くらい

  一部()

  // 50 行くらい
}

一部() {
  // オリジナル()
  // 修正版1()
  修正版2()
}

オリジナル() {
  // バグのあるあたり
}

修正版1() {
  // コピペして修正を試みたけどうまくいかなかったコード
}

修正版2() {
  // 試行錯誤中
}

まとめ


  • 「動いているものは触らない」
  • 影響を受けにくい修正の仕方を身につけよう!
  • 影響を受けにくい書き方を身につけよう!
Sign up for free and join this conversation.
Sign Up
If you already have a Qiita account log in.