code_smell


※上記の広告は60日以上更新のないWIKIに表示されています。更新することで広告が下部へ移動します。

refactor

コードの不吉な匂い

本書の役では匂いだが、どう考えても臭いだろう。

http://ja.wikipedia.org/wiki/%E3%82%B3%E3%83%BC%E3%83%89%E3%81%AE%E8%87%AD%E3%81%84

同じような発想を次の本からも得ることができそうだ。

こっちは持ってます。作者さんのサイトで、ネットでも文章が見れます。

重複したコード

複数のメソッドに同じ式がある=>メソッドの抽出

重複したコードが子供間で存在=>メソッドの抽出+フィールドの引き上げ

子供間でメソッド一部が重複 =>メソッドの抽出+Template Methodの形成

同じ結果で違うアルゴリズムを利用していた場合 =>[アルゴリズムの取り替え(139)]

まったく無関係のコードが重複している場合 => [クラスの抽出(149)]

長すぎるメソッド

単純に分割する=>[メソッドの抽出(110)]

一時変数を含む場合=>[問い合わせによる一時変数の置き換え(120)]

引数を取りすぎるメソッド=>[引数オブジェクトの導入(295)]or[オブジェクトそのものの受け渡し(288)]

それでもまだ一時変数が残る=>[メソッドオブジェクトによるメソッドの置き換え(135)]

分岐条件分を分割したい=>[条件記述の分解(238)]

巨大なクラス

原因:インスタンス変数のもちすぎ、いくつかの変数を構造化=>[クラスの抽出]

新しくできたクラスがサブクラスになりそう=>[サブクラスの抽出(330)]

新しくできたクラスがインスタンス変数をすべて利用していない=>[クラスの抽象化(149)][サブクラスの抽出(330)]

コード量が多すぎる=>[クラスの抽出][サブクラスの抽出]

クラスを分割したい=>[インタフェースの抽出(341)]

クラス間で同期すべきデータを持つ場合=>[観測されるデータの複製(189)]

多すぎる引数

データを1つの引数にして受け取ることが可能な場合=>[メソッドによる引数の置き換え(292)]

データをまとめて渡したい=>[オブジェクトそのものの受け渡し(288)]

オブジェクトのような論理的な固まりで渡せない=>[引数オブジェクトの導入(295)]

[引数オブジェクトの導入(295)]

変更の発散

 ある新しい機能追加に、変更する場所が3つも4つもでてきたのではコードを変更するリスクにさらされる。

[クラスの抽出(149)]で変更の理由ごとにクラスをまとめていくのが重要

変更の分散

 変更の発散とはちがい、1つのクラスの変更が他のクラスの変更へ波及する場合。

[メソッドの移動(142)][フィールドの移動(146)]を利用して変更部分を一つのクラスにまとめてやる必要がある。

属性、操作の横恋慕

あるメソッドが自分のクラスよりも他のメソッドをより多く叩く場合、古典的な間違いをおかしている。

[メソッドの移動]を行い、適切なオブジェクトへメソッドを移動させる。

メソッド内で様々なクラスのデータを参照している場合には[メソッドの抽出]をおこない、

[メソッドの移動]を行う。

原則に当てはまらないパターン

[Gang of Four]のStrategyとVectorパターン

Self Delegateパターン[Beck]

データの群

数個の関係のあるデータが現れたと感じた時=>[クラスの抽出(149)]

メソッドやシグニチャの場合 => [引数オブジェクトの導入(295)][オブジェクトそのものの受け渡し]

シグネチャとは、メソッドの名前、およびそのメソッドに対する引数の数と型により構成されています。
つまり、次の3つを組み合わせたものをシグネチャと呼びます。

    * メソッド名
    * 引数の数
    * 引数の型

http://sjc-p.obx21.com/word/js/signature.html

新たなオブジェクトを作るまでもないとためらう必要はない。

基本データ型への執着

個々のデータをプリミティブにしておかない=>[オブジェクトによるデータの置き換え(175)]

データが単純なタイプコードを示しており、その値が全体の振る舞いに影響しない=>[クラスによるタイプコードの置き換え]

タイプコード:Cppのenumeのようなやつ。

タイプコードによる条件分岐が行われていた場合=>[サブクラスによるタイプコードの置き換え(223)][State/Strategyによるタイプコードの置き換え(218)]

いくつかの属性をまとめられそうな場合=>[クラスの抽出]

基本データ型が引数に現れる場合=>[引数オブジェクトの導入(295)]

配列の面倒さから逃れたい=>[オブジェクトによる配列の置き換え(186)]

スイッチ文

オブジェクト指向ではスイッチ文が少なくなる

スイッチ文の分離をしたい=>個々のcaseについて[メソッドの抽出][メソッドの移動]

この時点で[サブクラスによるタイプコードの置き換え]または[State/Strategyによるタイプコードのおきかえ]を選択する。

継承構造がすでにできている場合=>[ポリモーフィズムによる条件記述の置き換え(255)]

同一メソッド内でわずかなケースに分岐するためにスイッチ文が使われていて、今後も条件が追加されないようなら書き換える必要はない。

[明示的なメソッド群による引数の置き換え(285)]

null値を見て分岐している場合=>[ヌルオブジェクトの導入]

パラレル継承

ある継承木中のクラスに付けられた

クラス名の頭の部分が別の継承木中のクラス名の頭の部分と同じ時、

その恐れがある<= (理解できてない)

[メソッドの移動][フィールドの移動]を利用して継承構造を廃止していく。

怠け者クラス

充分な仕事をせず、その見返りに合わないクラスは排除すべき。

サブクラスが充分な働きをしていないとき=>[階層の平坦化(344)]

ほとんど役立たないヘルパークラス=>[クラスのインライン化(154)]

疑わしき一般化

一般化したと思い込み、誰も使わないような凝ったしかけになっているとき。

大した動きを感じられない抽象クラス=>[階層の平滑化]

意味のない委譲の削除=>[クラスのインライン化(154)]

使われないパラメータを持つメソッド=>[引数の削除(277)]

わかりにくい抽象的な名前のメソッド=>[メソッド名の変更(273)]

あるクラスやメソッドがテストケースでのみ利用されている場合も、疑わしき一般化を考える必要がある。

一時的属性

インスタンス変数の値が特定の状況でしか設定されていないオブジェクトがある。

インスタンスのオブジェクトは値を保持するものなので、その利用の方法は適切でない。

=>[ヌルオブジェクトの導入]を利用すると条件分岐部分がスマートになる

(QQQ...methodへの一時変数の組み込みは無理なのか?)

一時変数の原因は複雑なアルゴリズムがいくつもの値を必要としている場合があるから。

たくさんのパラメータを渡したくなかったので、属性にしちまえとしたということ。

変数やその変数を参照しているメソッドを対象にして[クラスの抽出]を行うことができる。

MethodObjectパターン[Beck]になる。

メッセージの連鎖

同じ意味のメッセージがいくつものgetXxxxを介して伝わるようになっている時、

中間オブジェクトの関連が変わるたび、クライアントが影響を受けることになる。

=>[委譲の隠蔽(157)]を利用できる

実際にオブジェクトをつかっている部分を[メソッドの抽出(110)]で取りだし、

[メソッドの移動]で連鎖の端の方へ移動させていくようにする。

仲介人

委譲:

上司に対して会議に出席できるか尋ねると、手帳をみてから答える。

上司は本人が手帳を見ようが、PDAを見ようが関係のないことという。

これが過剰になるのはCode Smellが香しい。

=>[仲介人の除去(160)]

仲介人がわずか

=>[メソッドのインライン化(117)]

処理が付け加わっている場合

=>[継承による委譲の置き換え(355)]

不適切な関係

仲のよすぎるクラスは分離する。

[メソッドの移動]や[フィールドの移動]を利用する。

[双方向関連の単方向への変更(200)]が適応できないかを考えるのも良い

クラスが共通の趣味を持つ時=>[クラスの抽出(149)]を利用する

第三者のクラスを間に挟む=>[委譲の隠蔽(157)]

継承によって、子が親クラスと親密になりすぎる

=>[委譲による継承の置き換え(352)]を使う

クラスのインタフェース不一致

処理は同じでシグニチャ(関数の呼びだしの引数型、名前)のみが異なる場合=>[メソッド名の変更(273)]

これでうまくいかない場合はクラスにあっていない => [メソッドの移動(142)]を用いる

移動にともなって重複したコードが発生する場合 => [スーパークラスの抽出(336)]

未熟なクラスライブラリ

クラスライブラリに補助的に機能を付加させたい

クラスライブラリに追加したいメソッドがわずか => [外部メソッドの導入(162)]

追加される振る舞いが多くなりそう => [局所的拡張の導入(164)]

データクラス

getとsetしかもたないクラスはデータ保持用のStructライクなクラス。

他のクラスから過剰にアクセスをもらいがち。

=>[フィールドのカプセル化(206)]

コレクションクラスが属性にある場合、カプセル化されているかを調べる、

=>必要に応じて[コレクションのカプセル化(208)]を行う

変更されるとまずい属性がある場合 => [setメソッドの削除(300)]

振る舞いを追加できないか調べる、そのメソッドが存在する場合 => [メソッドの移動(142)]

そのメソッドが他オブジェクトのメソッドの一部になっている => [メソッドの抽出]後[メソッドの移動]

リファクタリング後場合に応じて [メソッドの隠蔽(303)]を行う。

相続拒否

継承の親子関係が必要なくなった時(一部分のメソッドしか利用してないなど)

兄弟を作成し、[メソッドの引き下げ(328)]、[フィールドの引き下げ(329)]を利用し、

使わない属性、操作を兄弟へうつす。

スーパークラスがより抽象に近づく。よくも悪くも伝統的になる。

サブクラスでスーパークラスの振る舞いは継承したいけども、

インタフェースは必要でない場合、真面目に考える必要がある。

=> [委譲による継承の置き換え(352)]を利用。

コメント

コメントが悪質なコードを解説する”消臭剤”として働くとき。

メソッド内に説明が必要な時 => [メソッドの抽出]

メソッドが適切に分割しているのにコメントが必要な場合 => [メソッド名の変更]

システムが特定の状態である時=>[表明の導入]

コメントが必要なコードの場合はリファクタリングを行い、コメントが不要になるコードを書くこと。

コメントの正しい使い方:

・不明点を書きとどめておく

・なぜこのような処理を選択したのかを書く

修正と他者との共有に役立つ。


疑問点メモ

このページにおける委譲とは?

State/Strategyについて

MethodObjectパターンについて


refactor_test