引数のデザイン
私が受ける仕事には、新規のプログラムを開発するのではなく、既存のプログラムの動作を変更したり、新しい機能を追加したりする「改修案件」が多く含まれています。 この改修案件では、新規開発と比べて、書かなければならないコードの量はずっと少なくなるのですが、その前段階にある「他人が書いたコードを読み解く作業」がなかなかのクセモノ。 つい先日も、JavaScript を使用したウェブページ上のアニメーションの改修作業を行ったのですが、まず既存コードがどのような動きをするのかを調べるのに大変な苦労をしました。
そのコードの動作を追うのが難しい理由は色々ありますが、最も大きな要因は、プログラム内で定義されている各関数の使い方が分かりづらいこと。 関数とは処理のシーケンスの中から関連する部分を分離して名前を与えたものであり、プログラムを読み解くための重要な手掛かりとなります。 しかしそれ故に、関数の設計が適切になされていないプログラムの読解は非常に厄介なものになってしまいます。
通常、プログラムを書くときは、処理をただ順番に書き下すのではなく、いくつかの「関数」に分割します。 これによって各ステップの処理が部品化され、再利用が可能になるわけですが、関数分けの効用はそれだけに留まりません。 たとえ、たった一度しか実行されない処理でも、これらを上手く関数化し、本体から分離することで、プログラムの可読性・保守性を向上させることができます。
flint blog: 関数分けの効用
簡潔すぎる引数名
先に述べたように、このプログラムはアニメーションを制御するためのものであり、始点および終点の座標を指定して要素を移動させる以下のような関数を含んでいました:
function move(elem, sx, sy, ex, ey, d, t){ ... }
さて、この関数 move が取る6つの引数には、それぞれ何を指定すればよいのでしょうか。
ちなみにこの関数 (に限らずプログラム全体に言えることですが) には一切コメントが付けられていません。
最初の elem には、動かしたい要素 (element) のIDを指定することは、実際に呼び出されている箇所を読めばすぐに分かります。
続く sx, sy および ex, ey も、それらに含まれる x
, y という文字から座標であること、また、s
は "start (開始)"、e
は "end (終了)" の意味であること、つまり、これらが移動の始点および終点の座標値であることも容易に見当が付くでしょう。
問題は d と t で、呼び出し箇所を見てみると、1000 とか 2500 のような数値が渡されていますが、それらの意味するところは不明確。 この状況において、d や t という引数名は明らかに「説明不足」です。 関数の内容を読み進めていくと、d は移動が開始されるまでにかかる時間、t は移動にかかる時間 (ともに単位はミリ秒) であることが分かりました。 d は "delay (遅延)"、t は "time (時間)" を表す名前だったというわけです。
関数というのは、その中身 (実装) について知らなくとも「どのように呼び出せばよいか」が分かるように定義 (厳密には「宣言」) するべきもの。 引数について言えば、名前の短縮を避けること、その使われ方についてコメントで説明をすることが望ましい記述の仕方となります。 引数 elem についても、要素の「何」を渡すのかが分かりにくい (例えば、要素を表すDOMオブジェクトかもしれない) ので、この操作対象を表す語は関数名に含めた上で、id のように変更するのが良いでしょう。 更に付け加えると、プログラミングの世界では、始点および終点を表すのに "src (source)", "dst (destination)" という語を使うという慣例があるので、それを取り入れるのも可読性の向上に役立つと考えられます。
/** 要素の移動 * @return なし * @param id 要素ID * @param srcX 始点 (x座標) * @param srcY 始点 (y座標) * @param dstX 終点 (x座標) * @param dstY 終点 (y座標) * @param delay 遅延時間 (単位: ミリ秒) * @param time 移動時間 (単位: ミリ秒) */ function moveElem(id, srcX, srcY, dstX, dstY, delay, time){ ... }
修正前と比較するとこの関数が「どんな動作をするか」「どのような引数を取るか」が分かり易くなったのではないでしょうか。 少なくとも、このコードを初めて読むプログラマが改修に要する時間は格段に短くなるはずです。
不適切な並び順
このプログラム内では、「移動」の他にも、要素が徐々に姿を現す「フェードイン」、逆に徐々に見えなくなる「フェードアウト」という効果が使われています。 それらを実行する関数の定義は以下の通り:
/** 要素のフェードイン * @return なし * @param id 要素ID * @param delay 遅延時間 (単位: ミリ秒) * @param time 移動時間 (単位: ミリ秒) */ function fadeinElem(id, delay, time){ ... } /** 要素のフェードアウト * @return なし * @param id 要素ID * @param delay 遅延時間 (単位: ミリ秒) * @param time 移動時間 (単位: ミリ秒) */ function fadeoutElem(id, delay, time){ ... }
これらの関数も、元の引数名は d, t となっており、コメントも付けられていなかったのですが、前節で関数 move に対して行ったのと同様の修正を施しました。 問題はこれらの関数を呼び出すコードにあります。 フェードイン/フェードアウトでは、要素は移動することなくその場で現れたり消えたりするため、関数 moveElem にある引数のうち srcX, srcY, dstX, dstY に相当するものが存在しません。 そのため、アニメーションのシナリオに沿ってオブジェクトを動かすコードを記述すると、次のような感じになってしまいます:
fadeinElem ("background", 500, 1000); fadeinElem ("character", 1000, 500); moveElem ("character", 0, 256, 704, 256, 1500, 3000); fadeoutElem("character", 4000, 500);
すべての関数に共通する delay と time への指定値が記述される位置が揃わずガタガタ。 これではタイミングの確認や調整が大変面倒です。 この問題は、moveElem の引数の順番を変更することで解決できます。
/** 要素の移動 * @return なし * @param id 要素ID * @param delay 遅延時間 (単位: ミリ秒) * @param time 移動時間 (単位: ミリ秒) * @param srcX 始点 (x座標) * @param srcY 始点 (y座標) * @param dstX 終点 (x座標) * @param dstY 終点 (y座標) */ function moveElem(id, delay, time, srcX, srcY, dstX, dstY){ ... }
fadeinElem ("background", 500, 1000); fadeinElem ("character", 1000, 500); moveElem ("character", 1500, 3000, 0, 256, 704, 256); fadeoutElem("character", 4000, 500);
タイミングに関する値の指定が前の二列に揃えられたため、大変見通しが良くなりました。
引数の並びを決める際は、それが属するグループ内の関数の並びとの整合性も重要になります。 原則として、共通するもの (それはグループ内のすべての関数にとって重要または本質的である蓋然性が高い) から記述するのがよいでしょう。
実は、C言語の入出力関数も、この引数の並び順に問題を抱えています。 ファイル記述子を指定してデータをストリームに書き出す関数のプロトタイプ宣言を幾つか書き出してみると:
int putc (int c, FILE* fp); int fputs (const char* str , FILE* fp); int fprintf(FILE* fp, const char* format, ... ); size_t fwrite (void* buf, size_t size, size_t nmemb, FILE* fp);
すべての関数に共通する fp の位置が、fprintf 以外では引数リストの末尾にありますが、前述の原則に従えば、これは先頭に置くべき引数です。 C言語ユーザとして、私はこの引数の並びについて以前より大変不満だったりするのですが、長い歴史のあるライブラリなのでもはや修正は不可能。こればかりは諦めるより他にありません。
誰のための見通しの良さか
私は折に触れて「プログラムは美しく・読みやすく書くべし」と主張していますが、同業者からすら、
- 「きちんと仕様通りに動けば、他はどうでもいいんだよ。」
- 「保守する人間 (大抵の場合は書いた本人 (つまり自分)) が分かればそれでいいじゃん。」
という反応が返ってくることがしばしば。 しかし現実には、今回の改修案件のように、書いた人間と面識すらない人間に向けて読解・修正の依頼が出されることが少なくありません。
コードが読みづらいものであれば、読解・修正には余分な手間、即ち工数が掛かることとなり、それは作業に対する見積・請求の金額にダイレクトに反映されることになるでしょう。 報酬を多く払えばそれで済むのならばまだマシな方で、納期直前に退職, 入院, 死亡, 失踪などの理由でコードを書いた人間がいなくなってしまった場合は、各方面に頭を下げてのスケジュール調整が必要となる上、下手をすれば賠償請求をされる可能性すらあります。
そんなわけで、普段はプログラムを読んだり書いたりすることのない発注者や管理職にとっても、納品される、あるいは部下が書き上げるコードの「質」に対して関心を払い、これの向上に努めることは、職務上の義務であると言えるでしょう。 ましてや、コードの読み書きを職能とするプログラマやシステムエンジニアについては、「動けばそれでいい」という意識の持ち主はプロ失格と見なすべきだろうと考える次第です。
関連エントリ
この「名前」がコンピュータにだけ必要なものであれば、
SX-5738
とかW17TF88
のように、「他と重複しない一意な」名前であれば十分ですが、プログラムを書く人間にとっても「名前」は重要な識別要素なので、通常は "Customer" とか "DeleteData" のように、自然言語 (通例英語) に近い「名前」を用います。flint blog: 類義語
このように、プログラムを機能のまとまりごとに関数化すると、それらの中により局所化されたコンテキストを持ち込むことができるようになり、結果として変数名などを簡略化できるというメリットがあります。 変数名が短くなったからといって、その役割が曖昧になることはありません。 むしろ可読性は向上すると言っても差し支えないでしょう。
flint blog: 関数分けの効用
書きあがったプログラムを眺めても、例えば関数名や引数の並びについて、何故そのようになったのか、ということは分からないようになっているのが一般的です。 それらすべてを挙げ連ね、その経緯について事細かに記述することも原理的には不可能ではありませんが、実際にそれを行ったとすれば、おそらく管理しきれぬ程に莫大な量のドキュメントができあがるはず。 その情報の洪水の中にあって、本来の仕様書や設計図に書き込まれていた重要な情報は、その他の数限りない、瑣末な情報に埋もれて見えなくなってしまうことでしょう。
けれども、ソフトウェア開発の知識・技術の本質とは、こうした「作る過程」における「瑣末なもの」の中にこそある、と私は考えています。
flint blog: 抜け落ちるもの
コメント