2
Help us understand the problem. What are the problem?

More than 5 years have passed since last update.

posted at

リーダブルコード~より良いコードを書くために~(2/3)

概要

8.巨大な式は(一目で理解できるくらいの大きさに)分割する

  • 式を表す変数(説明変数)を用意する
説明変数の例)適用前
if line.split(':').strip() == "root"

説明変数の例)適用後
username = line.split(':').strip()
if username == "root"
  • 管理や把握を簡単にする変数(要約変数)を用意する
要約変数の例)適用前

// ↓ 変数が5個もある * _ * 
if(request.user.id == document.owner_id){ 
    //ユーザーはこの文書を編集できる

}
要約変数の例)適用後

bool user_owns_document = (request.user.id == document.owner_id)
if(user_owns_document){
    //ユーザーはこの文書を編集できる
}

  • ド-モルガンの法則を使う(分かりやすくなる場合もある)
    • 例) not (a or b or c) <=> (not a) and (not b) and (not c)

if(!(file_exists && !is_protected)) ~ <=> if(!file_exists || is_protected) ~

  • 複雑なロジックと格闘する
    • 例) trueな条件を列挙していったら条件が複雑になった
      • => 一度立ち止まって反対方向から問題を考える
//範囲(begin,end)が重なる場合で分岐

//良くない例)
// ↓長い!
if( (begin >= other.begin && begin < other.end) || (end > other.begin && end <= other.end) || ... 
//範囲(begin,end)が重なる場合で分岐

//良い例) 反対方向から問題を考える => 重ならない場合
// ↓二行になるけどスッキリする
if(other.end <= begin) 
if(other.begin >= end)
  • マクロを使う(多用せよというわけでない)
void AddStas(Stats & add_form,Stats* add_to){
    // ↓フィールド名が違うだけでどの式も同じことをしている( add_to->set_xxx(add_from.xxx() + add_to->xxx()) )

    add_to->set_total_memory(add_from.total_memory() + add_to->total_memory());
    add_to->set_free_memory(add_from.free_memory() + add_to->free_memory());
    add_to->set_swap_memory(add_from.swap_memory() + add_to->swap_memory());
    .
    .

}
//マクロ適用後
void AddStas(Stats & add_form,Stats* add_to){
    #define ADD_FIELD(field) add->set_##field(add_from.field() + add_to->field())

    ADD_FIELD(total_memory)
    ADD_FIELD(free_memory)
    ADD_FIELD(swap_memory)
    .
    .
}

9.変数の読み易さ

  • 役に立たない変数を削除する
    • 複雑な式を分解していない変数、一度しか使ってない変数等
// ↓datetime.datetime.nowだけでも分かる
now = datetime.datetime.now() 
last_view_time = now
  • 制御フロー変数を削除する
    • 注) breakでは対処できない場合はその部分を新しい関数に移動させても良い
bool done = false;
while(/* 条件 */ && done){
.
.
    if(...){
        done = true;
        continue;
    }
}
/* 削除後 */
while(/*  条件 */){
.
.
    if(...){
        break;
    }
}


  • 変数のスコープを縮める
    • よく言われるのは「グローバル変数は使うな」.どこでどのように使われている追跡することが難しくなる
      • スコープのレベル(モジュール、クラス、関数、ブロックスコープなど)
    • クラスのメンバ変数もミニグローバルになっているとも言える
      • メゾットが沢山ある中で数個のメゾット内でしか使われないメンバ変数 => 関数内でローカル変数として用意する等して対応
    • 変数の宣言はその変数を使用する関数の直前の方が良い(関数の先頭で全部を宣言されると頭の中で変数を切り替え難い)
スコープ制限前)
PaymentInfo *info = database.ReadPaymentInfo();
if(info){
    ...
}
// (初見の人)「if文の外側で定義しているからまた後の方で使われるのかな?」
スコープ制限後)
if(PaymentInfo *info = database.ReadPaymentInfo() ){
    ...
}
// (初見の人)「infoのスコープはこのif文だけか、なるほど」

10.無関係の下位問題を抽出する

  • 大事!:大きい問題(関数やobj)を小さい問題(関数やobj)に分割(なるべく独立性を高くする)
    • 関数やコードブロックごとに「高レベルの目標はなにか」明快にしておく
    • コードの各行に対して「高レベルの目標に直接的効果があるか」? or 「無関係の下位問題を解決しているだけなのか?」
    • もし無関係の下位問題を解決しているコードが相当量(<-大事)あればそれらを抽出して別の関数になる(≒汎用コードを作る)
//無関係の下位問題の抽出前)

//高レベルの目標「与えられた地点から最も近い場所を見つける」
void findClosestLocation(lat,lng,array){
...
...
//「二つの地点の球面距離をするコード(相当量)」 <=無関係の下位問題
...
...


} 
//無関係の下位問題の抽出後)

//高レベルの目標「与えられた地点から最も近い場所を見つける」
void findClosestLocation(lat,lng,array){
...
...
//spherical_distance() <= 「二つの地点の球面距離をするコード(相当量)」の置き換え(うれしい恩恵:デバッグが容易、他の場面にも転用可)
...
...


} 

おわりに

※当初では2部構成で 「リーダブルコード~より良いコードを書くために~」をまとめる予定だったが以外と後半部分の量が多かったので,3部構成となってしまいそうです.ごめんなさい.

おまけ

書籍内には下記の書籍もいくつか紹介/推奨していた

1.Code Complete 第2版<上><下>-完全なプログラミングを目指して-
2.リファクタリング-プログラムの体質改善テクニック
3.プログラミング作法
4.達人プログラマー - システム開発の職人から名匠への道
5.Clean Code-アジャイルソフトウェア達人の技

Why not register and get more from Qiita?
  1. We will deliver articles that match you
    By following users and tags, you can catch up information on technical fields that you are interested in as a whole
  2. you can read useful information later efficiently
    By "stocking" the articles you like, you can search right away
2
Help us understand the problem. What are the problem?