サマリ
- 長く同じコードと付き合っているとリファクタリングしたいと思うはず。
- リファクタリングに専念できる時間をとれない、そのリファクタリングは本当に動作に問題がないかと問われて実施しきれない方向けの記事です。
- どういう状況ならばリファクタリングを進められるのか実施例とともに紹介します。
リファクタリングっていいですよね
リファクタリングとは以下のようなことを指します。
コンピュータプログラミングにおいて、プログラムの外部から見た動作を変えずにソースコードの内部構造を整理することである。また、いくつかのリファクタリング手法の総称としても使われる。
wikipedia リファクタリングより引用
リファクタリングによって得られるものはいろいろあります。
- 変更しやすさアップ
- 可読性アップ
- 処理速度アップ
- etc..
サービスやプロダクトとして、なんども変更を行うものを日々の業務で何度も見ていると、
ここはこうしておきたいなとリファクタリングしたくなるときはあるとおもいます。
リファクタリングをやろうとするとはばかる壁
チームであっても個人でもリファクタリングしたいという意識があって実際声にあがっても、
なかなか実施できないと思います。
やったらいいのはわかっている、価値はわかっている、けれども
- いつやるの？
- その修正が問題ないことをどう保証するの？
- その修正のテストはどうやるの？
といったことが課題として出てくると思います。
そういうこともあり、リファクタリングすることの優先順がさがり、手をつけられないままになるのかなと。
One more thingでやってみる
じゃあどうするのか？という話になりますが。優先順が高い案件でコードを修正するときに、修正箇所をただ修正するだけではなく、リファクタリングもやってしまいます。
※理由は後述しますが、行うリファクタリングは1つだけにします。
ポイントは修正した箇所のテストでリファクタリング内容に問題がないことを担保することです。
既存のテストコードがあれば、そのテストコードでテストできることに限定します。
もしテストコードがまったくない箇所であれば、追加でテストコードを書くか、手動でテストすることになると思います。
そのテストで、保証できる範囲でリファクタリングができる箇所はないか探ってみましょう。
これで、その修正は本当に問題ないのか？と言われることはありません。
また、よほど時間をかけすぎない限り、なぜ今やっているのか？と問われることはないでしょう。
ついででやるだけ？と思われるかもしれませんが、
このやり方はリファクタリングの価値が出やすいと考えています。
なぜならば優先順が高い修正箇所には、比較的その後も修正する機会はあると思うためです。
可読性がよくなっても、変更しやすくなってもその後読まれる機会も修正される機会も少ないならば
リファクタリングする価値はあまりありません。
一方で今後も修正する箇所であればリファクタリングしておく価値は高くなります。
実践できた例
修正対象のメソッドで使っているプログラミング言語らしい書き方にする
修正対象メソッドが以下のようなとき
def get_available_products(products: list[Product]):
"""販売可能な商品一覧を返す
:param products: 取扱商品
:rtype: list[Product]
"""
available_list = []
for product in products:
if product.is_discontinued:
continue
available_list.append(product)
条件の修正に加えて、Pythonらしくリスト内包表記で書いた。
def get_available_products2(products: list[Product]):
"""販売可能な商品一覧を返す
:param products: 取扱商品
:rtype: list[Product]
"""
return [
p for p in products if not p._is_discontinued or p._is_out_of_stock
]
get_available_productsのテストコードがあったので、リスト内包表記にしても問題ないことは自明となりました。
広い範囲の修正があったときにモジュールのインポート順を直す
Python2で実行していたスクリプトをPython3で実行できるように変更するときがありました。
このときモジュールのインポート順をコーディング規約に沿って変更しました。
言語仕様にもよりますが、モジュールのインポート順を変更すると挙動は変わる可能性があります。
Python2からPython3で実行できるように変更して問題ないことを確認するために、そのツールの全機能をひととおり動作確認することをテストしました。
そのため、インポート順を変更しても問題ないことを保証できました。
イメージとしてはこれを
import requests # サードパーティライブラリ
from random import randint # 標準ライブラリ
from oreorelib import something # 自分たちが作ったライブラリ
...
こう
from random import randint # 標準ライブラリ
import requests # サードパーティライブラリ
from oreorelib import something # 自分たちが作ったライブラリ
...
考えなければいけないリスク
さきほどリファクタリングは1つまでと書きました。
それは、いろいろ詰め込みすぎると、リスクが高くなるためです。
- バグを入れる可能性が高くなるリスク
- レビューする側が妥当な変更か確認しづらくなるリスク
などが主なものとして挙げられます。
やりすぎないように気をつけてください。
また、1種類といっても直列処理を並列処理にするといったインパクトの大きい変更はやや慎重にすることをオススメします（わたしの体験談より）。
レビュアーでも参加できるリファクタリング
ここまでだと、コードを書く立場だとそういうことできるけど最近レビュアーになることが多いし・・と一部の方はがっかりするかもしれません。
しかし、レビュアーもリファクタリングをすすめることはできると思います。
- 他の人のリファクタリングにはいいねという（称賛する）
- たとえtypoの修正やスペース1つ2つの修正であってもいいねしましょう
- こういうことの積み重ねでチームでのリファクタリングの価値について認識が擦り合っていくと思います
- ここ、こうしませんか？という、コードを書く
- 上述したような簡単にテストできる範囲でリファクタリングできそうなところをコメントで書いてみる
- リファクタリングできる箇所の具体例を共有していくことで他の人も気づきやすくなると思います
まとめ
普段すすめている修正でリファクタリングするポイントについてまとめて紹介しました。
大きなリファクタリングはこの方法ではやりづらいですが、少しずつコードを良くすることはできると思います。
個人であってもチームでも小さなリファクタリングを進められるようになると、よりレベルの高い大きなリファクタリングがすすめられます。
改善の一歩目として参考になれば幸いです。