カテゴリー : リファクタリング

 

条件判定による例外の置き換え

想定される状況なのに例外を使用している場合は「条件判定による例外の置き換え」を検討します。
 
たとえば「クーポンがある場合とない場合で割引価格を変更する」処理が以下のように定義されていたとします。
 
        // 販売価格を取得します
        private decimal GetSalePrice(bool hasCoupon, decimal listPrice)
        {
            try
            {
                return GetDiscountPrice(hasCoupon, listPrice);
            }
            catch
            {
                return GetDiscountPriceHasCoupon(listPrice);
            }
        }

        // 割引価格を取得します
        private decimal GetDiscountPrice(bool hasCoupon, decimal listPrice)
        {
            // クーポンがある場合
            if (hasCoupon == true)
            {
                throw new Exception();
            }

            return listPrice * 0.9M;
        }

        // 割引価格を取得します(クーポンあり)
        private decimal GetDiscountPriceHasCoupon(decimal listPrice)
        {
            return listPrice * 0.8M;
        }
 
見た目はまあまあ整っていますが、「クーポンがある場合」という正常系の処理に例外を使用している個所が問題です。
 
それでは「条件判定による例外の置き換え」のリファクタリングを適用してみましょう。
 
        // 販売価格を取得します
        private decimal GetSalePrice(bool hasCoupon, decimal listPrice)
        {
            // クーポンがある場合
            if (hasCoupon == true)
            {
                return GetDiscountPriceHasCoupon(listPrice);
            }
            else
            {
                return GetDiscountPrice(listPrice);
            }
        }

        // 割引価格を取得します(クーポンあり)
        private decimal GetDiscountPriceHasCoupon(decimal listPrice)
        {
            return listPrice * 0.8M;
        }

        // 割引価格を取得します
        private decimal GetDiscountPrice(decimal listPrice)
        {
            return listPrice * 0.9M;
        }
 
クーポンがある場合とない場合は「等しく発生する事象」ですので、「if – else」の条件判定で判断しそれぞれ割引価格取得用のメソッドを呼び分けています。
 

明示的なメソッド群による引数の置き換え

引数の特定の値に応じて、実行されるコードが分岐する場合は「明示的なメソッド群による引数の置き換え」を検討します。
 
たとえば割引価格のルールが「クーポンがある場合は定価の8割、ない場合は9割で販売する」だったとします。
 
上記を踏まえ「割引価格を取得する」メソッドを以下のように定義しました。
 
        // 割引価格を取得します
        private decimal GetDiscountPrice(bool hasCoupon, decimal listPrice)
        {
            // クーポンがある場合
            if (hasCoupon == true)
            {
                return listPrice * 0.8M;
            }

            return listPrice * 0.9M;
        }
 
これはこれで問題ないと思いますが、「明示的なメソッド群による引数の置き換え」を行うと引数の数を減らせます
 
        // 割引価格を取得します
        private decimal GetDiscountPrice(decimal listPrice)
        {
            return listPrice * 0.9M;
        }

        // 割引価格を取得します(クーポンあり)
        private decimal GetDiscountPriceHasCoupon(decimal listPrice)
        {
            return listPrice * 0.8M;
        }
 
このリファクタリングを行うと、条件分岐が呼び出し元に必要になります。「条件分岐をどこで行うのが適切か?」を考慮したうえで適用してください。
 

例外によるエラーコードの置き換え

メソッドがエラーコードを返すため、正常系と異常系が分かりづらい場合は「例外によるエラーコードの置き換え」を検討します。
 
たとえば「面積を取得する」メソッドが以下のように定義されていたとします。
 
        // 面積を取得します
        private int GetArea(int width, int height)
        {
            // 幅または高さが0以下の場合
            if (width <= 0 || height <= 0)
            {
                return 0;
            }

            return width * height;
        }
 
どうやら幅または高さに0以下を指定した場合は、エラーコードである「0」を返しているようですが、これが正常値なのか異常値なのか判断に迷うところですよね?
 
「幅または高さに0以下が指定された場合は異常」を明確にするため、「例外によるエラーコードの置き換え」を行います。
 
        // 面積を取得します
        private int GetArea(int width, int height)
        {
            // 幅または高さが0以下の場合
            if (width <= 0 || height <= 0)
            {
                throw new ArgumentException();
            }

            return width * height;
        }
 
呼び出し元で例外を処理する必要はありますが、正常系と異常系を切り分けることができました。
 

パラメータへの代入の除去

引数に対して代入している個所があったら、代わりに一時変数を使用します。
 
たとえば「四角を描画する」メソッドが以下のように定義されていたとします。
 
        // 四角を描画します
        private void DrawRectangle(int x, int y, int width, int height)
        {
            // x座標が0より小さい場合
            if (x < 0)
            {
                x = 0;
            }

            // y座標が0より小さい場合
            if (y < 0)
            {
                y = 0;
            }

            ・・・
        }
 
どうやらx座標とy座標には0以上の値を指定しなければならないようで、0より小さい値が指定された場合に0へ補正する処理が入っています。
 
しかし0を「パラメータへ直接代入」している個所が問題です。
 
上級プログラマーがパラメータの「値渡し」と「参照渡し」を適切に切り分け、可読性を考慮して行っている場合もありますが、たいていは「面倒くさい」という理由で行われたバグの源です。
 
後の修正でこのようなコードに出会った場合、プログラマーは「値渡し」か「参照渡し」かを確認し、自分が行う修正が呼び出し元まで波及するかまで調べなければなりません。
 
これではプログラマーがどんなに優秀でも保守性が向上しませんので、「パラメータへの代入の除去」のリファクタリングを行いましょう。
 
この例だと三項演算子を使うとキレイにまとめられそうです。
 
        // 四角を描画します
        private void DrawRectangle(int x, int y, int width, int height)
        {
            // x座標を補正
            int correctX = (x < 0) ? 0 : x;

            // y座標を補正
            int correctY = (y < 0) ? 0 : y;

            ・・・
        }
 
ここで重要なことは三項演算子を使ったことではなく、「値を補正するため、一時変数を使用した」ことですので、お間違いのないようよろしくお願いします。
 

ガード節とド・モルガンの法則

ガード節による入れ子条件記述の置き換え」のリファクタリングは、異常系のチェックを先に行い、returnや例外をスローすることで正常系の処理を単純化できますが、元のソースのつくりによっては非常にやりづらいリファクタリングです。
 
たとえば「年齢20歳以上、30歳未満の場合のみ登録する」といった処理があったとします。
 
        // 登録します
        private void Register(int age)
        {
            if (20 <= age && age < 30)
            {
                // 正常系の処理
                ・・・
            }
        }
 
このくらい単純な処理であれば正常系のif文だけでもいいかもしれませんが、たいていは複合条件になりますので、ここはキチンとガード節を作りましょう。
 
と簡単に申し上げましたが、正常系のif文から異常系のガード節を作成するのは難しい場合があります。
 
ここで有効なのが「ド・モルガンの法則」です。
 
ド・モルガンの法則

ド・モルガンの法則

 
手順としては「正常系を否定すれば異常系のガード節になる」ので、ソースを以下のように改修します。
 
        // 登録します
        private void Register(int age)
        {
            if (!(20 <= age && age < 30))
            {
                return;
            }

            // 正常系の処理
            ・・・
        }
 
これでは条件が複雑になっただけですので、ここに「ド・モルガンの法則」を適用します。
 
先ほどの図をプログラミング言語で表現すると、
 
!(A || B) == !A && !B
!(A && B) == !A || !B
 
となりますので、上記ルールに従って改修すると、
 
        // 登録します
        private void Register(int age)
        {
            if (!(20 <= age) || !(age < 30))
            {
                return;
            }

            // 正常系の処理
        }
 
ここまで分解できれば、後は条件一つずつ人間が分かりやすい形で表記すればよいので、
 
        // 登録します
        private void Register(int age)
        {
            if (age < 20 || 30 <= age)
            {
                return;
            }

            // 正常系の処理
        }
 
となります。ド・モルガンの法則を知っていれば、業務ロジックが分からなくてもガード節は作れます。
 
最初は難しいかも知れませんが、反復しているうちに使いこなせるようになりますので、がんばって覚えてください。
 

重複した条件記述の断片の統合

条件分岐全てに同じコードが存在する場合は、そのコードを条件分岐の外に移動します。
 
たとえば「価格を登録する」というメソッドが以下のように定義されていたとします。
 
        // 価格を登録します
        private void RegisterPrice(ConsumptionTax taxType)
        {
            // 内税の場合
            if (taxType == ConsumptionTax.TaxIncluded)
            {
                // 内税の価格を計算
                CalculateTaxIncludedPrice();

                // 登録する
                Register();
            }
            else
            {
                // 外税の価格を計算
                CalculateIncludeItemPrice();

                // 登録する
                Register();
            }
        }
 
ソースを見ると、登録するための「Register」メソッドが条件分岐全てに存在しますので、条件分岐の外へ移動します。
 
        // 価格を登録します
        private void RegisterPrice(ConsumptionTax taxType)
        {
            // 内税の場合
            if (taxType == ConsumptionTax.TaxIncluded)
            {
                // 内税の価格を計算
                CalculateTaxIncludedPrice();
            }
            else
            {
                // 外税の価格を計算
                CalculateIncludeItemPrice();
            }

            // 登録する
            Register();
        }
 
こうしておけば条件により分岐する処理と、条件によらず必ず実行しなければならない処理が明確になりますので、保守性が向上します。
 

引数の追加

メソッドに渡す引数が足りない場合は追加します。
 
「消費税を取得する」メソッドが以下のように定義されていたとします。
 
        // 消費税を取得します
        public decimal GetConsumptionTax()
        {
            return UnitPrice * 0.08M;
        }
 
これは内部で一時変数を使用していますので、なくすためのリファクタリングをしなければなりません。
 
今回は「引数の追加」で対応します。
 
        // 消費税を取得します
        public decimal GetConsumptionTax(decimal taxRate)
        {
            return UnitPrice * taxRate;
        }
 
ただし、やみくもに引数を追加してはいけません。引数が増えすぎるようなら「引数オブジェクトの導入」を検討してください。
 

引数の削除

メソッドに渡した引数が使われていない場合は削除します。
 
setメソッドの削除」で作成した絵本クラスに、「消費税を取得する」メソッドを追加したとします。
 
        // 税率
        private const decimal TAX_RATE = 0.08M;

        // 消費税を取得します
        public decimal GetConsumptionTax(int taxRate)
        {
            return UnitPrice * TAX_RATE;
        }
 
消費税を取得する「GetConsumptionTax」メソッドに税率「taxRate」の引数を渡していますが、消費税の計算式では絵本クラスで宣言した定数の税率を使用しているようです。
 
この場合、引数の税率は無意味なので削除します。
 
        // 消費税を取得します
        public decimal GetConsumptionTax()
        {
            return UnitPrice * TAX_RATE;
        }
 

setメソッドの削除

初期化の際に設定し、その後変更してはいけないフィールドにsetメソッドや、プロパティのsetアクセサが定義されている場合は削除します。
 
たとえば「絵本」を管理するためのクラスを作成し、コンストラクタで単価を設定したとします。
 
    // 絵本
    public class PictureBooks
    {
        // 単価
        private int _unitPrice;

        // コンストラクタ
        public PictureBooks(int unitPrice)
        {
            this._unitPrice = unitPrice;
        }
    }
 
その後、他のクラスで絵本の単価を知る必要があり、単価プロパティを用意したとします。
 
        // 単価
        public int UnitPrice
        {
            get
            {
                return this._unitPrice;
            }
            set
            {
                this._unitPrice = value;
            }
        }
 
思わずsetアクセサも定義してしまいましたが、これでは絵本クラスの外から単価を編集できてしまいます。
 
これはマズイので「setメソッドの削除」を用いてsetアクセサを削除しましょう。
 
        // 単価
        public int UnitPrice
        {
            get
            {
                return this._unitPrice;
            }
        }
 
こうしておけば絵本クラスの外から単価が編集されることがなくなります。
 
もし絵本クラス内での単価変更を許可する場合は、setアクセサのアクセス属性で制御します。
 
        // 単価
        public int UnitPrice
        {
            get
            {
                return this._unitPrice;
            }
            private set
            {
                this._unitPrice = value;
            }
        }
 
上記例では、単価を絵本クラスの外に公開しますが、設定は絵本クラスのみが行えます。
 

メソッドの隠蔽

クラス内部で使用するメソッドが「public」で宣言されている場合は「メソッドの隠蔽」を検討します。
 
たとえば「売価を設定する」メソッドが「public」で宣言されていたとします。
 
        // 売価を設定します
        public void SetSellingPrice()
        {
            ・・・
        }
 
このメソッドを調べてみると、クラス内部でしか使用されていませんでした。その場合は「メソッドの隠蔽」を用いてアクセス属性を「private」に変更します。
 
        // 売価を設定します
        private void SetSellingPrice()
        {
            ・・・
        }
 
必要最低限のメソッドのみ公開することで、後からクラスを利用する人が、どのメソッドを呼び出せばよいのか分かりやすくなります。
 
メソッドが派生クラスで使用されている場合は、アクセス属性を「protected」にしてください。
g h T
 6,908 Total Views