jfluteの日記

開発現場の定番悩み

とある機能実装のチケットを取って...
そのためのgitブランチを作成して...
さあ機能を実現するために実装して...
既存コードに追加修正入れるとして...

「既存コードめっちゃ直したい！」

目の前の変数名が微妙すぎるので...
メソッドの定義位置が微妙すぎるので...
追加修正で既存の名前が曖昧になるので...
ってゆーか放っておけないよこのコード...

って気持ちになることありませんか？
さて、どうしましょう？

相手はGithubプルリクの差分

その気持ちのまま直すと、Githubプルリクのときに...

本来の業務の修正とは別の差分が発生してしまい、本来の業務のコードのレビューがしづらくなる

というジレンマがあります。

gitコミットの履歴を振り返るときも、コミットメッセージの内容とは別の修正が入ってしまってノイズになってしまいます。

なので、そういったリファクタリングは、

別チケットにして別のタイミングで別のブランチでやるのが良い

というのが月並の正論です。

リファクタリングチケットタワー

ですが往々にして、

後回しされたリファクタリングは消化されずに積み重なり、大量の残チケットとなって余計に手を付けられなくなりがち

です。

そびえ立つタワー。どんどん高くなっていく。

リファクタリングチケットありすぎて、やれる気がしないからやらない。
...

あと、つどつどリファクタリングチケットを新たに登録するのもわりと面倒。

で、「変数名ちょっと変だなって思ったくらいだから...わざわざチケットにするほどか？...見なかったことにしよう」ってことでスルーされる問題も。

リファクタリングアプローチパターン

そういったとき、アプローチするパターンどんなものがあるでしょう？

A. 差分気にせず豪快リファクタリング

うおーーーー、目に入った瞬間にリファクタリング！

Githubプルリクでは、業務実装とリファクタリングが一緒くた！
まあ、レビューワーがんばってパターン。

その中でもちょっと選択肢:

A-1. コミットも分けずにもう一緒にコミット
A-2. コミットだけは分ける

ただ、コミット分けても、レビューワーがプルリクが全体差分から見てたらあまり意味がないので、コミットで「リファクタリング」と「業務実装」を分けてるのでそこ見てくださいってのを伝えないと。

B. 差分ノイズ少しなら許容リファクタリング

程度問題、

全くレビューできないってレベルじゃなければ混ぜちゃっていいよ

と。

ただ、そのボーダーが個人依存で、やり過ぎたり遠慮し過ぎたりするかも。
...

一方で、これはさすがにってものだけ別チケットに登録。

なので量のあるリファクタリングは結局後回しなので、それをいつ消化する？ってのは、結局次の "C" パターンと同じく悩まないと。

C. 絶対に別チケットリファクタリング

プルリクで目に入る差分は、純粋な業務実装だけ！

と。

別チケットに登録。登録作業を面倒くさがらない！些細なことでも登録しよう。

そして、後回しのリファクタリングは絶対にやる！

C-1. 業務チケット終了後にすぐにやる
C-2. 後で時間のあるときにやる (たぶんやらない)
C-3. 週1のチーム全体リファクタリングタイムでやる
C-4. 月1のチーム全体リファクタリングタイムでやる
C-5. 年1のチーム全体リファクタリングタイムでやる
C-6. 人生1のチーム全体リファクタリングタイムでやる

本当にやるかな？
...

C-7. リファクタリングチケットタワーをそびえ立たせる

したら、チケットの登録作業をしてる分だけ時間の無駄。

D. 今でも後でもそもそも直さない

いやこれは残念だが...

いやいや、でもハッキリしていることだけは良いことかも。

ハッキリしてないことよりは迷わない。

E. ハッキリしない

大抵のデフォルト。金縛りに合うような感じ。

「jfluteさん、どうすればいいですかね？」って相談が...

まずはチーム方針を確認

チーム方針があるかどうか？まずそれを確認。

あれば、それにそのまま従う。

なければ、話し合ってチーム方針を決めようと提案。
...

あっても、リファクタリングチケットタワーがそびえ立ってたら方針を考え直そうと提案。

jflute個人的には

理想ではないことを前提に、

B. 差分ノイズ少しなら許容リファクタリング

が現実的でオススメかなとは考えています。
(UnitTestがあること前提として)

多少の個人ブレは許容、ちょっと多すぎなら「ちょっと大きいかな、次回からは」って話をするだけでいい。

もちろん、大正論の

C. 絶対に別チケットリファクタリング

がスムーズに実現できるならそれが一番ですが、この場合、

C-3. 週1のチーム全体リファクタリングタイムでやる

が大前提かなと思います。

例えば、金曜日の16時〜17時だけは、リファクタリングチケットだけやりましょうタイム！みたいな。(隔週でもいいし)

でなければ、リファクタリングチケットタワーがどんどん積み上がるだけかなって想像してしまいます。

業務チケット終わったら、どう考えても次の業務チケットやりますよ、きっと。後で時間ができたら、さらにその次の業務チケットをやりますよ、きっと。

みんなよほどリファクタリングが好きじゃなければ。

// リファクタリングという行為が好きか？ | jfluteの日記
https://jflute.hatenadiary.jp/entry/20220328/loverefactor

人がどんどん既存コードを嫌いになる

既存コードに手も出せないとなると、

既存コードは自分には関係ないもの感...

既存コードに自分の責任は一切ないもの感...

「既存コードと関わる仕事いやだなー」

既存コード好きになれる環境(状態)じゃないと、なかなかその現場に長く滞在しないと思います。

TicketBooth

既存コード:

// OneDayパスポートのみ販売
// (quantittty変数は、OneDay在庫)
public class TicketBooth {

    private int quantittty = 10;
    ...

    public void buyOneDayPassport(...) {
        --quantittty;
        ...
    }
}

　↓↓↓ 機能追加

// TwoDayパスポートの販売も追加
// (twoDayQuantity変数は、TwoDay在庫)
public class TicketBooth {

    private int quantittty = 10;
    private int twoDayQuantity = 10;
    ...

    public void buyOneDayPassport(...) { 
        --quantittty;
        ...
    }
    public void buyTwoDayPassport(...) {
        --twoDayQuantity;
        ...
    }
}

TicketBoothクラスに、

「OneDayパスポート」の在庫を示す quantittty変数があって...

「TwoDayパスポート」の在庫を示す twoDayQuantity変数を新たに追加したら...

既存コードの quantittty変数の名前はどうします？

(OneDayしかなかったらシンプルな名前を付けていた)
(しかも、既存の変数スペルミスしてる)

javatry (じゃゔぁとらい) より:
https://dbflute.seasar.org/ja/tutorial/javatry/index.html