LoginSignup
2977
3218

More than 5 years have passed since last update.

@hirokidaichi(広木 大地)in株式会社Rector

新人プログラマに知ってもらいたいメソッドを読みやすく維持するいくつかの原則

Last updated at Posted at 2014-05-09

エンジニア組織を強くするための本を出版しました

Qiitaでエンジニアリングをめぐる様々なコミュニケーションの問題とその解決策や考え方を書いてきた。それらの背後にあるエッセンスをこの度書籍として出版するに至りました。

エンジニアリング組織論への招待
～不確実性に向き合う思考と組織のリファクタリング

この書籍は、エンジニアリングを「不確実性を削減する」という第一原理で捉え直し、様々なエンジニアリングとその間のコミュニケーションをめぐる現象を説明していくものです。

あわせて読みたい

この記事について

この記事は、新人向けの研修内容を再編集してお送りします。
この記事の内容は絶対のものではありません。重要なことは読みやすいコードを書くためにプロジェクトやチーム、言語のサポート状況で異なってきます。メンバー間でどのような合意を形成していくかというのがもっとも重要です。一つの目安や議論のベースとしてお使いください。

あといろんな言語の話を混在して書いていますが、なんらかの疑似言語だとおもって読んでください。

原則

今回、メソッドをきれいに保つための原則としては以下の５＋１つを紹介します。

  • 論理演算子の数を減らそう
  • ブロックネストの数を減らそう
  • 変数のスコープを短く保とう
  • 変数への再代入を減らそう
  • 説明的でないループを減らそう
  • これらを自動チェックしよう

論理演算子の数を減らそう

たとえば、１つの条件分岐にたくさんの&&や||、!といった論理演算子がたくさん含まれていると条件文の内容を把握するのは難しくなります。ゼロから書き始める場合はそんなことしないよと思うかもしれませんが、もともとある関数に条件を追記したい場合などにそれらの条件を変更したくないという作用が働いて、論理演算子の数が増えてしまう場合があります。

論理演算子の数はできれば１つまで
if( A and B) {


}

二つ以上の論理演算子があるととたんに読みにくくなる

if( A && B || C ){

}

if( not A or not B) ){

}

また、論理演算子の優先順位は言語ごとに多少の違いがあり、読み違いやバグの温床になります。

ドモルガンの法則

単純な論理演算子の数の削減方法としては、ドモルガンの法則があります。
http://ja.wikipedia.org/wiki/ド・モルガンの法則

if( !A or !B ){}
if( !A and !B and !C and !D){}

このような条件文は、

unless( A and B ){}
unless( A or B or C or D ){}

に書き換えることができます。
これにより、notの数が減り読みやすくなります。

これは、コードを初めて書く場合や読む場合は自然に理解できるため、適応しやすいものではあります。

一方、ごく自然なメソッドの成長の仕方として、if(!A )という式に対して、!Bのケースも追加したいという場合、if(今までの条件に or !B)とするほうが自然でわかりやすいように錯覚してしまいがちです。

今、書いているひとにとってわかりやすい書き方と、その後
はじめてこのコードに触れる人にとって理解しやすい書き方はこういうケースでは異なってきてしまいますので注意が必要です。

このようなケースでドモルガンの法則を意識した追加がなされるためにも
unlessのような否定をともなった予約語がある言語であれば、賢く使っていくのがよいでしょう。

メソッドの切り出し

たとえば次のようなコードがあった場合、

if( (A && B) || (C && D)){
    :
    :　副次的なケース
    return result
}
  :
  : メインの処理
  :
return result

副次的なケースのみを関数に切り出して、次のようにすると読みやすくなります。

return this.subMethod(...) if( A and B );
return this.subMethod(...) if( C and D );
 :
 : メインの処理
 :
return result

if修飾子、unless修飾子がある場合は、それらをうまくつかって
処理を切り出すとorでつながれたケースを分解し、みやすくなります。

ある処理から機能の追加時にメソッドを切り出して、その部分のunitテストを書いて、レガシーコードの浸食を押さえるテクニックを スプラウトメソッド などと言ったりします。

説明的変数の導入

条件文が必要以上に複雑になってしまった場合、次のように説明的な変数を導入してあげることでもコードは読みやすくなります。

説明的変数を導入する
var isHeapOverCapacity = (A and B) or (C and D);
if( isHeapOverCapacity ){

}

メソッド/クラスの切り出し(その２)

複雑な条件式が多用されている場合、それらを切り出したクラスやメソッドを定義するのも必要です。

if( int(status/100) == 4 || int(status/100) == 5 || err != null) {

}

こういったケースに対して、

if( this.hasError() ){

}

であるとか、

if( response.hasError() ){

}

のように説明的なメソッドや関数、クラスに切り出すことでその場所でやっていることは理解しやすくなります。また、関数として切り出した場合、||による接続は、次のように縦に並べることができます。

function hasError(){
    return isClientError(status) or
            isServerError(status) or
            ( this.err != null );

}

function hasError(){
    return true if isClientError(status); 
    return true if isServerError(status);
    return (this.err != null ):
}

複雑になった場合は縦にif文をならべることでわかりやすくなるかもしれません。

ブロックネストの数を減らそう

ブロックネストとは、if文やfor文などのブロックを伴う制御構文によって{}の階層を持つことです。

if(...){ // 1
    for(...){ // 2
        for(...){  //3
            if(){       //4
                :
            }
            if(){ 
                :
            }
            switch(){
                :
            }
        }
    }
}

このようにネストが深くなればなるほど、コードは入り組んで読みにくくなる。

ガード節

ネストを下げる一番シンプルな方法は、一番外側のif文などを反転させることです。

if(...){
  ....メインの処理....
}
throw Error;

このように、特定の条件を満たさない場合に例外やエラーを返すような関数の場合、

throw Error unless(...);
... メインの処理...

のように書き換えることができる。
これによって一番外側の大きなifは切り崩すことができる。

制御構造をデータ構造に変換する

制御構造が複雑になっているとき、データ構造の側をわかりやすくすることで、コードをシンプルにすることができます。

ディスパッチテーブル

for文の中のswitch
for(var i=0;i<array.length;i++){
    switch(array[i]){
        case 'a':doSomethingForA();break;
        case 'b':doSomethingForB();break;
                ：
                ：
        default:doDefault();
    }
}

このような処理があった場合、このswitch文はdispatchテーブルを持つことで、シンプルに記述できます。

var dispatchTable = {
    a : doSomethingForA,
    b : doSomethingForB
            :
};

for(var i=0;i<array.length;i++){
    (dispatchTable[array[i]] || doDefault)();
}

このように制御構造をデータ構造に変換することで、煩雑なコードをシンプルに書き換えることができます。

直積を作る

たとえば、次のようなケースを考えてみよう。

２つのfor文がnestされている
tx = %w(1 2 3 4 5)
ty = %w(a b c d e)

for x in tx 
  for y in ty
    p([x,y])
  end
end

for文が２つ重なっている状態になっている。
これもたとえば、(1 a)(1 b) ..(1 e)(2 a)..(5..e)みたいなペアを先に作ってあげることで、１つのfor文にかえることができます。

先に直積を作ってしまう
tx = %w(1 2 3 4 5)
ty = %w(a b c d e)

for point in tx.product(ty) 
  p(point)
end

このようにすることでfor文のネストの数をさげることができます。
先に列挙することでパフォーマンスの問題がある場合や途中でbreakするようなケースであれば、


for point in tx.lazy.flat_map{|x| ty.lazy.map{|y|[x,y]} } 
  p(point)
end

このように遅延評価で直積をとってあげることもできます。
flat_mapをつかって直積をとる形は、scalaであれば以下のように書けます。

var tx = ( 0 to 5 )
var ty = ('a' to 'e')
for(i <- tx;j <- ty ) println(i,j)

for式が直積のシンタクスシュガーとして動作します。

制御構造をデータ構造にすべきとき/すべきでないとき

制御構造をデータ構造に変換することは常に正解という訳ではありません。

もっと単純な解決策は、複雑な制御構造の一部を別の関数やメソッドとして切り出すことです。しかし、「制御構造の成長の仕方」に注目し、拡張されるべきポイント（成長に対して開かれているところ）がどこかと考え、見つけ出せたときにデータ構造に変換することやさらに複雑であれば、クラス構造に変換することが最良の策になるでしょう。

この制御構造をデータ構造やクラス構造に変換するノウハウとして、Gofの振る舞いに関するデザインパターンが挙げられる。

Chain of responsibilityパターン 、Commandパターン、Iteratorパターン、Visitorパターンなどがわかりやすい例となるでしょう。
http://ja.wikipedia.org/wiki/デザインパターン_(ソフトウェア)

変数のスコープを短く保とう

変数のスコープとは、あるところで宣言された変数名が有効な範囲のこと。
http://ja.wikipedia.org/wiki/スコープ

関数が長くなってくると、様々なところで宣言された変数が入り組んでくることがある。

function XXX(){
    var x = ...;
    var y,z = ...,....;

        :

    var p,q,r = ...,...,...;

        :
        :

}

このようになってくると、どこで何が書き換えられるかということを調べないと、
長くなった処理の関数への切り出しが難しくなってしまう。

たとえば、呼び出すたびに１つ増えていくカウンターを考えてみましょう。

var count = 0;
function counter(){
    return count++;
}

本来、count変数は他の人から触れてほしくないのに、外のスコープにあります。
なので、誰かがcount = 10のようにしたら、counter関数の呼び出すたびに増えるという契約は崩れてしまうのです。

var counter = (function(){
    var count = 0;
    return funtion(){return count++}
})();

このようにスコープを作り、クロージャを作ることでスコープを狭めたりできます。
副作用がある場合には、オブジェクトやクロージャ、メソッド切り出しをすることで、
変数のスコープが広くなりすぎないようにすることができます。

変数への再代入を減らそう

同じ変数に対して、破壊的な操作をしたり、再代入するなどして値を変える処理が増えると、関数は読みづらくなります。

たとえば、次の図のように
スクリーンショット 2014-05-09 18.06.38.png
一つのメソッドの中で４回値が書き変わってしまった場合、それぞれ途中途中で変数のさしている意味や事柄が変わってきます。

今はどういう状態なのか正しく理解しないと書き換えられないコードになる。
それぞれ、変化がある場合はそれを説明する変数名をつけることでコードは読みやすくなる。

三項演算子または値を返すif文

ありがちな再代入を許してしまうケースとして、

var result;
if(isActive){
    result = "ACTIVE";
}else {
    result = "INACTIVE";
}

次のようなコードを考えてみよう。
resultの値を決めるという目的なのだから、

var result = (isActive)? "ACTIVE":"INACTIVE";

と書いた方がわかりやすい。
三項演算子は、わかりにくいとされることもあるが、それは条件が複雑になってきた場合の話で、条件が複雑な場合は、先に述べたようにいずれにせよわかりづらいので、こういったケースでは使っていく方がよい。

rubyなどif式自体が返値を持っているような場合はそれをつかうのもいいでしょう。

def status(isActive)
  if isActive
    :ACTIVE
  else
    :INACTIVE
  end
end

説明的でないループを減らそう

最近の言語であれば、リストに対する一般的な処理は、なんらかのライブラリとして提供されていることが多い。３つのステートメントをとるfor文は汎用的であるがゆえに意味を取りにくい。パフォーマンスセンシティブでないケースであれば、できるかぎりリスト処理をつかって意味の明確なループを使いましょう。

また、foldLeft,reduce系統の処理（つまりリストから１つの値を取り出す処理）は、変数の再代入を減らしてくれる効果があります。

var sum = 0;

for(var i=0,l=numbers.length;i<l;i++){
    if(numbers[i]%2 == 1 ) continue;
    sum += numbers[i]
}

このように再代入をともなう処理も、

var sum = numbers.filter(isOdd).sum();

このように書くことができれば再代入も減り、意味も明確になります。

numbers = (1..10)
def odd?(n)
  (n % 2 == 1) 
end
p numbers.select(&method(:odd?)).inject(:+)

他にもall,any,none,include?など本来ならばループを伴う処理もリスト関数を使うことでシンプルにわかりやすくなります。

これらを自動チェックしよう

今まで挙げたような関数をきれいに保つ原則を適応していくと循環的複雑度:CCという値が小さくなります。

この値が増えるほど、必要とするテストケースの数は増えていく。
JavaScriptであれば、このサイトjsmeterでCCの計測もインタラクティブにできる。

他の言語でもjenkinsなどのCIにのせて計測していくことで関数を清潔に保つことがしやすくなる。

だいたい、言語によるが10~15くらいの値を超えないようにしていくのがいいでしょう。

参考文献

新人プログラマに正月休み中を使って読んでみてほしい技術書をセレクトしてみた。

エンジニアリング組織論への招待～不確実性に向き合う思考と組織のリファクタリング

2977
3218
8

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
Sign upLogin
2977
3218