LoginSignup
2
0

More than 3 years have passed since last update.

だめなコードを修正する!

Last updated at Posted at 2019-06-28

tl;dr

仕事仲間から渡されたPythonコードをみて愕然とした。一関数1000行、いわゆるサブルーチン化も十分なされてない。極めつけは、関数をコピペして部分的に書き換えた二系統のコードを実装。いわゆるアンチパターン。自分でフルスクラッチで書き直したほうが早いもかもしれないが、コードの管理責任は彼にあり、最悪コードが二重化する恐れも。ということでこちらでリファクタリング(コード整理)してから返すという形にした。今回は、そのときの手順を公開する。一応、特定の言語に限定しないようには書いてみる。

事前準備

1.構成管理ツールを準備しソースをコミットする
リファクタリングするのだから変更履歴を参照できるように、svnやgitで管理したほうがよい。サーバーが準備できなくてもsvnならローカルで運用もできる。
2.リファレンスの処理結!果データをつくる
本当はテストコードを書くのが好ましいが、短いプログラムだとテストコードを書くほどではない場合もある。それでもリファクタリングするなら、改変前後で処理結果が変わらないことを確認できるように処理結果データを作る。
3.差分比較ツールをインストールする
Windowsなら、WinMergeがオススメ。
http://winmerge.org/?lang=ja
4.コードフォーマッタをインストール、整形し、構成管理にコミットしておく。
c++ならclang format。pythonならYAPF。統合開発環境ならツールに組み込まれているか、プラグインになっていることが多い。ちなみに、Visuaul Studio 2017をc++で使う場合には、独自のコードフォーマッタとclang formatがあるが、clang formatがオススメ。.clang-formatファイルをプロジェクトディレクトリに置くことで、プロジェクトのコードスタイルを他メンバーに強制できる。本題に戻って、リファクタリングするときは、最初にカッコやインデント、タブの書式をそろえるべき。あとからフォーマットを変えるとなると差分比較が難しくなってしまう。そこでコードフォーマッタを使うことを勧める。

今回のリファクタリング目的

A. 処理内容を理解しやすいコードに改変する
B. 冗長なコードを集約してコードの記述量を減らす(コードのメンテナンスが容易な状態にする)

リファクタリング

1.各所ある複数ステップからなる共通の処理を関数化し、関数コールに置き換える

2.関数の行数が多い場合、一塊の処理を関数化し、関数コールに置き換える

まあ、コメントや空行を除いても100行以内か。100行を超えるのは、論理的に意味のある複雑なif分や、どうしても必要な巨大なswicth文が存在する場合か。関数化することで処理に適切な名前をつけることになり(つけるべきであり)、コメントを追加しなくても、全体の処理の流れが短い行数で把握しやすくなる。

3.使わない関数、変数はコメントアウトする

本当に使わないことが確定したらコードが除去する

4.標準ライブラリに同様の機能の関数やクラスがある場合、標準ライブラリを使うように書き換える

標準ライブラリーにあることを知らずに自作したり、標準ライブラリーの拡充により、同じ機能の標準関数が存在するということがあり、基本は標準ライブラリを集約していくことが望ましい。高速化した実装が使われていたり、デグレの危険を犯したくない場合は、あえて書き換えないという判断もある。

5.コメントを書き足す

初見でコードの挙動がわからず、読み込んだらわかったということがある。そのときはコメントを足しておこう!

6.不要なコメントを消すか書き換える(意外と重要!)

だめなコードの割にコメントをまめに書く人がいる。ただ読めばわかるコードのコメントは不要。不要と思ったら思い切って消してしまえ!あと、リファクタリングの結果コメントに矛盾が発生する場合もある。それも削除するか正しい内容に書き換える。

7.新しい構造体やクラスを作ったり、用意されている構造体やクラスを使うことによって変数を集約する

例えば、座標を表現する場合、x,yを独立して扱うのでなく、x,yをメンバ変数にもつPoint型を介して扱うなど。

8.似た目的で使われる変数は集約する

例えば状態管理が二重になっている場合は一元化する。

9.大きすぎる構造体やクラスを分割する

10.メンバ関数化を考える

グローバルな関数もメンバ関数化できないか考える。

11.関数の引数を最適化する

一つの基準として、関数を呼び出すたびに変化する値は単一の引数とする。それ以外は構造体にパックして構造体の変数にするか、メンバ変数とする。ここは個人の好みによるところが大きいが、全体のコードで一貫性をもたせたほうがよい。

12.変数名、関数名、クラス名、ファイル名を、適切な意味のあるものに書き換える

意味のない名前をつけていることがあり、意味のある名前につけかえることで、コードを理解しやすくさせる。

→数十行書き換えたら、(コンパイルが必要な言語はコンパイルし)、実行し、処理結果データ比較して変化がないことを確認してコミット。もし結果が違ってしまったらコミットしてあった前のバージョンのコードと変更点を比較して問題発生の箇所を特定する。基本は1から順にやっていくとよいが、ある程度リファクタリングが進むとやり残しを気づいたところから改変していくことになる。もちろん、テストコードが存在するならリファクタリングしながら適宜実施しましょう。

雑記

だいたいだいこんなことをやった!
c++のリファクタリングはよくやるが、pythonのリファクタリングは初めてやった。個人的にはコンパイル言語のほうが、まずコンパイルができることで一定のコード妥当性が確認できるので、リファクタリングしやすいと思った。まあ、どんな言語でも他人に提供するなら、コード読みやすく、メンテナンスが容易な状態にしてから渡すようにすべき。コーディング規約を決めてそれに則って書くとなおよいと思います。

2
0
0

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
2
0