リファクタリングのすゝめ

リファクタリングって？

リファクタリング (refactoring) とはコンピュータプログラミングにおいて、プログラムの外部から見た動作を変えずにソースコードの内部構造を整理すること。

euremoはコードを「読みやすく」「書き直しやすく」「書き加えやすく」するために行うことだと思っている。もっともらしい言葉に言い換えると、「可読性」「保守性」「拡張性」ってところだろうか。これと違う考え方で、性能向上のためのリファクタリングもあるのだけど、性能と可読性はときに相容れない。euremoはそういうときは、まずは可読性を取る。その上で、もう読まない、もう手を入れない、ブラックボックス化できるコードに関しては、性能向上を図ってもいいだろう。最適化するのは、自分が何を書いているのか理解してからで遅くない。

さて、それじゃあ可読性ってなんだろうか。

コードの可読性

読みやすいコードと一言で言ってもむずかしい。読んで理解しやすいコードが常に読みやすいとは限らないからだ。たとえば処理が長くなると読みにくい。だからといって、処理を関数に分割すればいいかというと、連鎖的に関数を呼び出すようなコードは「処理を追いにくい」ということで嫌われたりもする。
しかしながら、処理を追いやすいかどうかや、処理を理解しやすいかどうかよりも重要なことがある。
「処理の目的を理解しやすいかどうか」、これこそが一番重要だと考える。

LOCAL = A + B

足し算は合計を求めるのが目的だから、すぐに分かる。

LOCAL = DAY % 7

このコードが剰余を求めていることは分かるが、なぜ剰余を求めたいのだろう？　しかしこれくらいなら、少し考えればDAYという変数名と、7という数字から、7で割った余り、つまり曜日を求めたいらしいと分かる。だが、もっといいのは何も考えなくても見ただけで「曜日を求めたい」と分かるコードだろう。

LOCAL = GET_DAYOFWEEK()

少々やりすぎではないかと思うかもしれないが、曜日を数値で取得してそれで終わり、というケースより、曜日を文字として表示したい、というケースの方が多いだろうから、

IF LOCAL == 0
    LOCALS = 日
ELSEIF LOCAL == 1
    LOCALS = 月
　：
　：
　：
ELSE
    LOCALS = 土
ENDIF
LOCALS += "曜日"

なんていう処理が続いているかもしれない。「いやここはSELECTCASEの方が」とか「文字列配列を使って添字にLOCALを渡せば」とかそういう次元でなく、

LOCALS = %DAYOFWEEK_STRING()%

こう書きたいことの方が多いわけだ。
その上で、DAYOFWEEK_STRINGの内部としてどういう処理がいいかと考える。この際IF～ELSEIFかSELECTCASEか文字列配列かは置いておいて、曜日を数値で求める処理をどうするか、だ。言うまでもない。文字列として取得したい場合もあれば、数値として取得したい場合もある。文字列として取得する場合も、内部では数値で取得するわけだから、数値で取得する関数は少なくとも2箇所以上で呼び出されることになる。2回以上使うコードなら、関数にしてしまってもまったく問題ない。大体の場合、やりすぎということはないのだ。

さて、せっかくなのでDAYOFWEEK_STRINGのコードも書いておこう。

@DAYOFWEEK_STRING
#FUNCTIONS
    VARSET LOCALS
    SPLIT "日;月;火;水;木;金;土", ";", LOCALS
    RETURNF LOCALS:(GET_DAYOFWEEK()) + "曜日"

@GET_DAYOFWEEK
#FUNCTION
    RETURNF DAY % 7

関数の細分化にデメリットがないわけではない。たとえば、関数呼出のオーバーヘッドが積み重なれば性能は低下する。そういうオーバーヘッドが無視できない分野や環境もあるわけだ。逆に、性能の低下が問題にならないか、問題になるほどの性能差が出ないなら、可読性を優先するだろう。
これとは別に、可読性を損なうという理由で関数の細分化を嫌う人もいる。が、そういう場合はだいたい関数の命名が悪いのだと思っている。処理の目的が分かるなら、関数を追っていく必要はないのだから。

処理は短く
処理の目的毎に関数化
関数名は目的が分かるように

このあたりが、可読性向上のキーになるだろう。

コードの保守性

可読性の話が済んだところで、保守性の話に進もう。
保守、と聞くとなんだか「変更を避けよう」というような感じを受けるが、この場合保守したいのはコードそのものではなくコードの正常性である。なので、むしろ異常なコードはどんどん改変していきたいことになり、改変しやすいコードが望ましい。すなわち保守しやすいコードということになる。メンテナンス性と言い換えても構わない。

ところで、メンテナンスするのは誰だろう？
多くの場合作った本人が行うが、別人が代行することも多い。たとえば複数人で共同して作っていて、作った本人がメンテナンスできないなら、それ以外の人がメンテナンスすることになるだろう。また、プログラミング分野ではよく言われることだが「三日前の自分は別人」であり、過去に書いたコードというのは作った本人にとっても、後から読み返すときに結構な努力を強いられるものだ。「はて、このコードはどういう処理をしているんだったか」と思うことは決して少なくない。
そこで重要なのが、前項の可読性である。可読性の低いコードは、すなわち保守性も低い。保守性を確保するためには、可読性の向上が必要不可欠になってくる。可読性の話を先にしたのは、そういうわけだ。
しかし、可読性だけで保守性を確保できると思ったら、それもまた間違いである。

次のようなコードは、変更に大きな労力を必要とする。

IF ABL:従順 == 1
    ;従順Lv1のときの処理
ELSEIF ABL:従順 == 2
    ;従順Lv2のときの処理
ELSEIF ABL:従順 == 3
　：
　：
　：
ENDIF

このコードは、決して読みにくいコードではない。しかし、「ABL:従順の名前をABL:親密に変えたい」と思ったら、該当の箇所すべて修正しなければならないのだ。
一般的に言って、定数リテラルを直接参照するコードは保守性が低い。このコードは以下のように書き換えた方が変更に耐えるものとなるだろう。

LOCALS = "従順"
IF ABL:LOCALS == 1
    ;従順Lv1のときの処理
ELSEIF ABL:LOCALS == 2
    ;従順Lv2のときの処理
ELSEIF ABL:LOCALS == 3
　：
　：
　：
ENDIF

文字列リテラルを直接使っている箇所は大抵の場合読みやすいのだが、文字列自身を変更したい場合に困ることが多い。多少可読性を損ねてでも、文字列リテラルを変数に置き換えた方が、後々の苦労を軽減させる。
さて、変更箇所の多さがそのまま労力に繋がるわけだから、変更のしやすさとは、変更箇所を減らすことだと言ってもいい。
そういう意味では上記のコードはまだ不充分だろう。下記のコードはより保守性に優れている。

LOCALS = "従順"
SELECTCASE ABL:LOCALS
CASE 1
    ;従順Lv1のときの処理
CASE 2
    ;従順Lv2のときの処理
CASE 3
　：
　：
　：
ENDSELECT

何が減ったか？
答えは「ABL:LOCALS」の数である。同じものを書く回数を減らせば、「後から変更しなければならないかもしれない箇所」も減る。当然、書く回数が少ないということはバグ混入のリスクが少ないという意味でもあるから、場合によっては「後から変更しなければならないかもしれない可能性」さえも減らすこともできる。
この観点で見れば、処理を関数にまとめることも保守性向上に役立っていると言える。同じ処理を関数にまとめることで、変更箇所を減らすことができるからだ。

保守性のキーは、

読みやすく
変更箇所を少なく
同じことを何度も書かない

だろう。

一方で、処理を単純化すればするほど失われていくものもある。

コードの拡張性

コードの保守性とコードの拡張性は似ているようでぜんぜん違う。どちらもコードの変更しやすさだが、その目的が違うのだ。一方は正常性を保守したいのであり、もう一方は機能を拡張したり追加したいのだから。そして機能拡張や機能追加はその性質上エンバグととても仲が良く、バグは保守性の天敵だったりする。
コードの保守性と拡張性は、ときとして相反する要素になりえる。その一つが、処理の単純化に伴う拡張性の低下だ。

LOCALS = "従順"
SELECTCASE ABL:LOCALS
CASE 1
    ;従順Lv1のときの処理
CASE 2
    ;従順Lv2のときの処理
CASE 3
　：
　：
　：
ENDSELECT

保守性に優れると書いたコードだが、このコードは拡張しにくい。「従順がLv4で、かつ奉仕精神がLv3以上のとき」みたいなものを書こうとすると、どうだろう。

　：
　：
　：
CASE 4
    IF ABL:"奉仕精神" >= 3
        ;奉仕精神が3以上のときの処理
    ELSE
        ;従順が4のときの処理
    ENDIF
CASEELSE
    ;5以上の処理
ENDSELECT

CASEの内側に更に条件分岐を組み込む必要が出てしまった。以前のコードならば、こう書けばよかった。

LOCALS = "従順"
IF ABL:LOCALS == 1
    ;従順Lv1のときの処理
ELSEIF ABL:LOCALS == 2
    ;従順Lv2のときの処理
ELSEIF ABL:LOCALS == 3
    ;従順Lv3のときの処理
ELSEIF ABL:LOCALS == 4 && ABL:"奉仕精神" >= 3
    ;従順Lv4で奉仕精神Lv3以上のときの処理
ELSEIF ABL:LOCALS == 4
　：
　：
ENDIF

どちらがいいとは一概に言えない。条件を単純化できるならば素直に保守性を取ればよい。条件を複雑化する可能性があるなら、その余地は残しておいた方がいいかもしれない。

かといって、拡張性を見越してコーディングを行うべきなのだろうか？

さて、「処理を共通化すると変更の影響範囲が大きくなるから拡張しにくい」という意見がある。
これは「No」だ。
共通化した処理は、同じ目的のもと共通化されているはずである。であるならば、処理結果が変わるような変更はそもそもすべきでないのだ。別の目的があるのならば、新たに処理を書き起こすべきである。関数として共通化してあるのなら、処理を変えたい場所では呼び出す関数ごと変えればいい。むしろ元のコードを損なうことなく新たなコードを書き加えられるのだから、拡張性が高いと言える。

たとえば、可読性の項に書いたDAYOFWEEK_STR関数は、曜日の書式を変更することができない。しかし、書式を指定したいならば別の関数を用意して、それを呼び出すべきである。

@DAYOFWEEK_STRING
#FUNCTIONS
    ; 日本語で取得して後ろに曜日をつけて返す
    RETURNF DOW_STRING() + "曜日"

; ARGは英日指定; 0:日本語 1;英語
; ARG:1は略記するかどうか; 0:略記しない 1:略記する（英語のみ有効）
; ARG:2は文字種別; 0:頭文字のみ大文字、1:全大文字、2:全小文字（英語のみ有効）
@DOW_STRING, ARG, ARG:1, ARG:2
#FUNCTIONS
    VARSET LOCALS
    SELECTCASE ARG
    CASE 1 ;英語で取得
        SPLIT DAYOFWEEKS_EN(), ";", LOCALS
    CASEELSE ;日本語で取得（デフォルト）
        SPLIT DAYOFWEEKS(), ";", LOCALS
    ENDSELECT
    LOCALS = LOCALS:(GET_DAYOFWEEK())
    IF ARG == 1
        SIF ARG:1 == 1
            LOCALS = %SUBSTR(LOCALS, 0, 3)%
        SELECTCASE ARG:2
        CASE 1 ;大文字に
            LOCALS = TOUPPER(LOCALS)
        CASE 2 ;小文字に
            LOCALS = TOLOWER(LOCALS)
        ENDSELECT
    ENDIF
    RETURNF LOCALS

@DAYOFWEEKS
#FUNCTIONS
    SIF LOCALS == ""
        LOCALS = "日;月;火;水;木;金;土"
    RETURNF LOCALS

@DAYOFWEEKS_EN
#FUNCTIONS
    SIF LOCALS == ""
        LOCALS = "Sunday;Monday;Tuesday;Wednesday;Thursday;Friday;Saturday"
    RETURNF LOCALS

かなり多機能化したが、更にここから「英語表記を全角にしたい」とか「仏語にしたい」「"太陽の日"、"軍神の日"といった独自の曜日表現を使いたい」などといった別の機能を追加したくなるかもしれない。今後はそういった機能拡張を行いたければDOW_STRINGの内部に処理を追加すればいい。そのためにはちょっと今のDAYOFWEEK_STRINGFは重たすぎる。以下のように書き換えると、より拡張しやすい。

@DOW_STRING
#FUNCTIONS
    VARSET LOCALS
    SPLIT DAYOFWEEKS(), ";", LOCALS
    RETURNF LOCALS = LOCALS:(GET_DAYOFWEEK())

@DOW_STRING_EN, ARG, ARG:1
#FUNCTIONS
    VARSET LOCALS
    SPLIT DAYOFWEEKS_EN(), ";", LOCALS
    LOCALS = LOCALS:(GET_DAYOFWEEK())
    SIF ARG == 1
        LOCALS = %SUBSTR(LOCALS, 0, 3)%
    SELECTCASE ARG:1
    CASE 1 ;大文字に
        LOCALS = TOUPPER(LOCALS)
    CASE 2 ;小文字に
        LOCALS = TOLOWER(LOCALS)
    ENDSELECT
    RETURNF LOCALS

大文字、小文字の処理は英語にしか必要なく、日本語に関係ない機能を切り離すことができた。全角・半角の処理も曜日の日本語表記には関係してこないので、日本語部分に今後手をつける必要はほとんどない。
このように、機能を追加すれば処理は複雑になるが、後のことを考えると結局機能単位で処理が完結している方が拡張しやすいことになる。機能ごとにまとめ、シンプルに、簡潔に記述することで、後から処理を書き加えやすくなる。後から書いたものが複雑になったならば、その都度シンプルに、簡潔に直していく。その繰り返しこそが、拡張性を高め、維持するために必要だ。
最初から拡張性を意識して書くことは、かえって拡張性を損なうのだ。

可読性・保守性にこだわりすぎない
拡張性にこだわりすぎない
複雑になったら単純化

多機能化は処理の複雑化を伴うことが多い。処理を単純化しすぎれば、複雑な処理を組み込みにくくなる。無理やり処理を追加すれば、かえって複雑になりかねない。しかし、複雑になったならばその都度分解して単純化すればよい。はじめからある種の「ゆるさ」を残しておくことが拡張性のためには良いことも多いが、あまりにもゆるいコードでは拡張も元も子もない。
確かに、せっかくシンプルに書いたとしても拡張した結果のコードが読みにくく修正しにくいことは多い。
が、生まれたてのコードはえてしてそういうものだ。繰り返し繰り返し、よりよいコードを目指して書き直していくしかない。

そして「コードの最適化」へ

コードの改善を行っていくと、いつしか「もう変更する必要も、変更が加えられる可能性もない」というコードが現れるようになってくる。こうなったら、もう可読性も保守性も拡張性も何もない。人間のことを考えるのをやめて、機械と対話するときが来たわけだ。
思う存分、ミリ秒マイクロ秒のパフォーマンス向上、あるいは1byteでも多くのメモリ消費節約に力を注いでも構わない ;-)

参考書籍

開発のお供にどうぞ。

このページを編集するこのページを元に新規ページを作成

印刷する

コメント（0）

リファクタリングのすゝめ - euremoのeramemo 先頭へ

コメントをかく

名前	ユーザIDを使用しないで書き込む	ユーザーIDを使う	ログインする
画像コード	画像に記載されている文字を下のフォームに入力してください。
備考	「http://」を含む投稿は禁止されています。
本文
利用規約をご確認のうえご記入下さい

euremoのeramemo