iimon TECH BLOG

iimonエンジニアが得られた経験や知識を共有して世の中をイイモンにしていくためのブログです

「リファクタリング(第2版): 既存のコードを安全に改善する」を読んで

■ 前書き

こんにちは、なかむらです。

入社当初から入力チームに入って1年4ヶ月が過ぎました。

入社最初は全体像がわからず動くコードをひたすら書いていましたが、段々と見えてくることも増えてきた気がします。

最近新しいメンバーが複数名加入したこともあり、より可読性の高いコードを書けるよう気をつけるようになりました。

そこで、今回は「リファクタリング」を勉強会のテーマにしました。

練習問題を3つ作ったので、良ければ腕試しにお使いください。

リファクタリングとは?

リファクタリングとは、既存のコードを変更せずに、コードの構造を変更して、コードの理解や保守を容易にすることです。

■ なぜリファクタリングを行うのか?

【 実際に体験した問題点 】

・開発者の移動によってなにをしているかわからないコードが残っていて、読み解くために時間がかかる。

・長い関数はスクロールしているうちに変数が何を指すものか、何をしたいのか忘れる。

リファクタリングのメリット 4つ 】

・コードが理解しやすくなる

・コードの崩壊を防ぐ

・バグを見つけやすくする

・長期的に早く開発できる

【 気をつけること 】

・テストを書くこと

 リファクタリングを行う時に、最初に行うことはちゃんとしたテスト群を作成することです。

 人間が作業する以上、間違いを犯す可能性があります。

 テストフレームワークを作成し、コマンド一つで実行できるようにします。

・新機能の実装が遅くなる

リファクタリングは、コードベースがどれだけ美しいかではなく、純粋に経済的な基準で図られるべき

自社ブログですが、「質とスピート」に着目した松田さんの記事が参考になりました。

結局「質とスピード」、大事なのはどっちやねん? - iimon TECH BLOG

■ 練習問題 1

難易度:★

printDetails関数をprintOwingの内部に作ってconsole.log部分をまとめてください。

□ 問題

function printOwing(invoice) { 
    printBanner();
    let outstanding = calculateOutstanding();
    
    //print details
    console.log(`name: ${invoice.customer}`); 
    console.log(`amount: ${outstanding}`);
}

答え&変更理由
□ 答え

function printOwing(invoice) { 
    printBanner();
    let outstanding = calculateOutstanding(); 
    printDetails(outstanding);
    
    function printDetails(outstanding) { 
        console.log(`name: ${invoice.customer}`); 
        console.log(`amount: ${outstanding}`);
    } 
}

□ 変更理由

printDetails を作成することで、printOwing 関数がよりシンプルになり、責務が明確になります。

また、ネストすることで親関数の変数やコンテキストに直接アクセスできます。

これにより、引数を増やすことなく、必要な情報にアクセスできます。

■ 練習問題 2

難易度:★★★

グローバル変数に対して、アクセサ関数(getDefaultOwner, setDefaultOwner)を作成して直接データを書き換えないようにしてください。

□ 問題

let defaultOwner = { firstName: "Martin", lastName: "Fowler" };

// 次のように参照
spaceShip.owner = defaultOwner;

// 次のように更新
defaultOwner = { firstName: "Rebecca", lastName: "Parsons" };

答え&変更理由

□ 答え

let defaultOwner = { firstName: "Martin", lastName: "Fowler" };

function getDefaultOwner() {
    return Object.assign({}, defaultOwner);
}

function setDefaultOwner(arg) {
    defaultOwner = arg;
}

// 次のように参照
spaceShip.owner = getDefaultOwner();

// 次のように更新
setDefaultOwner({ firstName: "Rebecca", lastName: "Parsons" });

// -----検証-----
console.log(defaultOwner);
// {firstName: 'Rebecca', lastName: 'Parsons'}

// プロパティからのデータ変更を防げる
spaceShip.owner.firstName = 'hogehoge';

console.log(defaultOwner);
// {firstName: 'Rebecca', lastName: 'Parsons'}

□ 変更理由

問題点は、defaultOwnerがグローバルスコープに存在するため、どこからでも変更できるところです。予期せぬ場所でデータが変更されるリスクがあります。



let defaultOwner = { firstName: "Martin", lastName: "Fowler" };
spaceShip.owner = defaultOwner;
console.log(spaceShip.owner);
// {firstName: 'Martin', lastName: 'Fowler'}

// 予期せぬデータ変更
defaultOwner = { firstName: "Rebecca", lastName: "Parsons" };
spaceShip.owner = defaultOwner;
console.log(spaceShip.owner);
// {firstName: 'Rebecca', lastName: 'Parsons'}

これに対し、データの取得や設定を行うアクセサ関数を作成することで、データ変更の一元管理ができます。

また、関数内での追加処理(例えば、バリデーションやイベントのトリガー)を行うことができるため、単なる代入以上の処理が行われます。

さらに、getDefaultOwnerでObject.assign()を使用して、defaultOwnerのコピーを返しています。

Object.assign() - JavaScript | MDN

これにより、返されたオブジェクトを外部で変更しても、defaultOwner自体は影響を受けません。

もし以下の書き方をしていたら、オブジェクトの参照を直接外部に公開するため、外部コードが意図せずオブジェクトを変更するリスクがあります。

let defaultOwner = { firstName: "Martin", lastName: "Fowler" };
function getDefaultOwner() {

   // Object.assign()を使用しない場合、データ参照を返す
    return defaultOwner;

}

function setDefaultOwner(arg) {
    defaultOwner = arg;
}


// 使用例
spaceShip.owner = getDefaultOwner();

// ここでdefaultOwnerが意図せず変更される
spaceShip.owner.firstName = "Rebecca"; 

console.log(defaultOwner); 
// {firstName: 'Rebecca', lastName: 'Fowler'}

■ 練習問題 3

難易度:★★★★★

readingOutsideRangeのパラメータをstation, rangeの2つにしてください。

□ 問題 

const station = { name: "ZB1",
    reading: [
        {temp: 47, time: "2016-11-10 09:10"},
        {temp: 53, time: "2016-11-10 09:20"},
        {temp: 58, time: "2016-11-10 09:30"},
        {temp: 53, time: "2016-11-10 09:40"},
        {temp: 51, time: "2016-11-10 09:50"}
    ]
};

// 範囲外の測定結果を検出する関数
function readingOutsideRange(station, min, max) {
    return station.reading.filter(reading => reading.temp < min || reading.temp > max);
}

// readingOutsideRangeの呼び出し元
const alerts = readingOutsideRange(station, operatingPlan.temperatureFloor, operatingPlan.temperatureCeiling);

答え&変更理由

□ 答え

const station = { name: "ZB1",
    reading: [
        {temp: 47, time: "2016-11-10 09:10"},
        {temp: 53, time: "2016-11-10 09:20"},
        {temp: 58, time: "2016-11-10 09:30"},
        {temp: 53, time: "2016-11-10 09:40"},
        {temp: 51, time: "2016-11-10 09:50"}
    ]
};

// データをまとめるためのクラスを作成
class NumberRange { 
    constructor(min, max) {
        this._data = {min: min, max: max}; 
    }
    get min() {return this._data.min;}
    get max() {return this._data.max;} 
    
    contains(arg) {return (arg >= this.min && arg <= this.max);}
}

// NumberRangeインスタンス range作成
const range = new NumberRange(
    operatingPlan.temperatureFloor,
    operatingPlan.temperatureCeiling
);

// NumberRangeインスタンスメソッドを実行
function readingsOutsideRange(station, range) { 
    return station.readings.filter(r => !range.contains(r.temp)); 
}

const alerts = readingOutsideRange(station, range);

□ 変更理由

元の readingOutsideRange 関数は、温度の下限と上限を個別のパラメータとして受け取っていました。

これにより、関数の呼び出し時に複数のパラメータを渡す必要があり、コードが冗長になりがちです。

NumberRange クラスを使用することで、温度範囲を一つのオブジェクトとして扱い、関数のパラメータリストを簡素化ができます。

また、温度範囲の概念を明確になり、コードの可読性が向上します。

さらに、新しい範囲が追加される場合でも、クラスを拡張するだけで対応できるため、メンテナンス性も向上します。

■ 感想

リファクタを進めると、一気に修正したくなるときがあります。

しかし、タスクと並行して少しずつ修正することと、経済的な基準も忘れないよう気を付けていこうと思います。

この本の気になった部分をつまみ食いした感じなので、実務をしながらまた読んで理解していきたいです。

■ 募集

弊社ではエンジニアを募集しております。

ぜひカジュアル面談でお話ししましょう!

ご興味ありましたら、ご応募ください!

Wantedly / Green

■ 参考

Amazon.co.jp: リファクタリング(第2版): 既存のコードを安全に改善する (OBJECT TECHNOLOGY SERIES) : Martin Fowler 著, 児玉 公信, 友野 晶夫, 平澤 章, 梅澤 真史: 本

[図解][解説動画付き] リファクタリングの原則 #コーディング - Qiita