LoginSignup
1
1

More than 3 years have passed since last update.

@n350071(石垣 尚紀)

設計としてのリファクタリング <原則とコードのニオイ25種>

Last updated at Posted at 2019-07-17
1 / 35

2019/07/19時点

ある勉強会のネタです。（この記事自体が大きすぎて、全体としては未完です）

「しろふねさん、勉強会で発表しますか？」と尋ねられたので急遽、１年半前の読書メモから書き起こしています。

はじめに

コードを書いていて、ぐちゃぐちゃになってしまったり、引き継いだプロジェクトのソースコードの手続きが絡み合ったいい感じのパスタになっていて、それが 品質と納期に多大な影響を及ぼしている と感じた経験はないでしょうか？

私にもその経験があり、できることなら品質と納期を高いレベルにもっていき、顧客に価値あるソフトウェアと届け続けたい..と願うエンジニアです。この問題と戦いはじめて７年程経ちました。

一時期は「アジャイルでやればうまくいく」と思い、プロジェクトのプロセス改善に集中した時代もありました。

しかし、決定的に欠けていたものがありました。それはリファクタリングです。リファクタリングなき設計とは、一度書いたコードを書き直さないということであり、それはつまり、はじめから最適な設計をしなければならない重厚な事前設計を意味します。私はアジャイルの前提である進化的設計が欠けた状態で、アジャイルのプロセスを回そうとしていたのです（できることを増やすXPの実践）。

SIer時代に得たPMで、プロジェクトとしては成功しましたが、マーチン・ファウラー的に言えば「プロジェクトのスタミナが減少しているという感じていました。

そこで、Rubyです。

  • オブジェクト指向言語であり、型を意識するというよりは、オブジェクトに話しかけるようにメッセージに集中できること
  • 開発現場に設計だけする人を見かけないので、実装=設計行為となり、オブジェクト指向開発をやるには良い環境

でも、Rubyを使えば必ずオブジェクト指向に書けるわけではない。
やりたいことは、オブジェクト指向なRubyを書いて柔軟に成長できるアプリケーションを書きたいっていうことです。

そこで、この３冊です。今回は３冊目のリファクタリングに焦点を当てていきます。英語の本ですが、翻訳が間違っていたらすみません。

本の構成

  • リファクタリングの原則
  • コードの悪臭 25臭
  • リファクタリング実践７タイプ(83手)
    • メソッドの構成(20)
    • 機能をオブジェクト間で動かす(6)
    • データの構成(17)
    • 条件文をシンプルにする(9)
    • メソッドコールをシンプルにする(16)
    • 一般化する(11)
    • 大きなリファクタリング(4)

※実際の目次通りではありません

2. リファクタリングの原則

Why Should You Refactor ?

リファクタリングによって、設計と可読性、品質を向上し、バグを減らす。
そしてこれらが生産性を上げる。

だから、やるべき。

When Should You Refactor ?

  • always!

（息を吸うように）

The Rule of Three

  1. 関数を追加したときに（設計の完成度を上げる）
  2. バグを修正するときに（コードの見通しを良くする）
  3. コードレビューをするようにリファクタリングをする

When Shouldn't You Refactor?

  • 現状のコードが動かないとき（書き直そう）
    • 大きな塊をきちんとカプセル化されたコンポーネントに置き換えられそうな場合には、リファクタリング vs 作り直しの間で悩んでもよい。
  • 締切間近。
  • 学術的だったり、自分の信条においてリファクタリングするとき（それはコストだ）。

Refactoring and Design

アプリケーションを書くときに、いきなり最適な設計をできるだろうか？ほとんどの場合は、頭の中にあるものを、まず書いて、動かして、それからリファクタリングでシャープにしていくものじゃない？

ただ、それでも効率的とはいえない。ゆえにXPでも少しの設計を先にやる（CRCカードなどを使って）。

もちろん、もしもリファクタリングしないとしたら、BDUP（重厚な事前設計）には大きなプレッシャーと、あらゆる変更のコストが高くなる（のでリファクタリングで進化的設計を行おう）。

Building Tests

  • テストを書いてから作業しよう

本題に入る前のまとめ

  • リファクタリングが必要ないように最初から最適な設計をする == Up Front Big Design(重厚な事前設計）
  • 少しずつ最適な設計に近づけていく == オブジェクト指向が活きる漸進的な設計行為

3. コードの悪臭 25

悪臭のカテゴリ

  1. 肥満系
  2. オブジェクト指向違反系
  3. 抵抗勢力系
  4. 不用品
  5. 結合系
  6. その他

Bloaters(肥満)

Long Method

長い行数のメソッド。コメントが必要と感じるくらいに長ければ、それは臭いのサイン。

対策
  • まず最初に
  • パラメータや一時変数が沢山ある
    • → Replace Temp with Query
    • → Replace Temp with Chain
    • → Replace Temp with Method Object
  • 条件式
    • → Decompose Conditional
  • ループ
    • → Closure Method

Large Class

多すぎるインスタンス変数や、長いメソッド

対策
  • Extract Class
  • Extract Module
  • Extract Subclass

Primitive Obsession(プリミティブへのこだわり）

  • 小さなオブジェクトの代わりにプリミティブを使っている
対策
  • まずは、
    • → Replace Data Value with Object
  • 変数が条件によって変わるのであれば、
    • → Replace Type Code with Polymorphism
    • → Replace Type Code with Module Extension
    • → Replace Type Code with State/Strategy
  • インスタンス変数のグループがあるならば、
    • → Extract Class
  • 配列を扱っているならば、
    • Replace Array

Long Parameter List

メソッドの引数が3ないし4以上の場合..

対策
  • パラメータをメソッドの戻り値として得られそうなら
    • → Replace Parameter with Method
  • あるオブジェクトから得たデータを詰め込んでいる
    • → Preserve Whole Object (そのオブジェクトを詰め込む)
  • いつくかの振る舞いのないデータが使われている（ `Order.book(start_day, end_day, num, break_first?)
    • → Introduce Parameter Object

Data Clumps

3,4個のデータが揃っていくつかの場面で現れる..

対策
  • → Extract Class (they should be into the extract class)
  • → Introduce Parameter Object
  • → Introduce Preserve Whole Object

Object Orientation Abusers (オブジェクト指向違反系の悪臭)

*今回はskip (以前の読書メモのまま早送り..）

Case Statements

  • Signs and Symptoms
  • You can find case statement
  • Treatment
  • You should consider polymorphism
    • Extract Method to extract the case statement
    • Move Method to get it onto the class where the polymorphism is needed
    • Replace Type Code with Polymorphism
    • Replace Type Code with Module Extension
    • Replace Type Code with State/Strategy
  • exception
  • a few cases that affect a single method
    • Replace Parameter with Explicit Method
    • Introduce Null Object

Temporary Field

  • Signs and Symptoms
  • An object which an instance variable is set only in certain circumstances.
  • Treatment
  • Use Extract Class with these variables and the methods that require them. The new object is a Method Object.

Refused Bequest

  • Signs and Symptoms
  • Subclasses pick just a few methods and data of their parents.
  • Treatment
  • If inheritance makes no sense and the subclass really does have nothing in common with the superclass, eliminate inheritance in favor of Replace Inheritance with Delegation.
  • If inheritance is appropriate, get rid of unneeded fields and methods in the subclass. Extract all fields and methods needed by the subclass from the parent class, put them in a new subclass, and set both classes to inherit from it ( Extract Superclass ).
  • The hierarchy is wrong

Alternative Classes with Different Interfaces

  • Signs and Symptoms
  • Methods do the same things but method name are different.
  • Treatment
  • Rename Methods to make them identical in all alternative classes.
  • Move Method, Add Parameter and Parameterize Method to make the signature and implementation of methods the same.
  • If only part of the functionality of the classes is duplicated, try using Extract Superclass. In this case, the existing classes will become subclasses.
  • After you have determined which treatment method to use and implemented it, you may be able to delete one of the classes.

Change Preventers(抵抗勢力系）

Divergent Change (変更が分岐してアチコチへ伝搬する)

  • Signs and Symptoms
  • When one class is commonly changed in different ways for different reasons.
  • Treatment
  • identify everything that changes for a particular cause
  • Extract Class

Shotgun Surgery

  • Signs and Symptoms You whiff this when you make a kind of change, you have to make a lot of little changes to a lot of different classes.
  • Treatment
  • Move Method
  • Move Field
    • into single class(find or create)
      • find it
      • create it
      • Inline Class

Parallel Inheritance Hierarchies

  • Signs and Symptoms
  • a special case of shotgun surgery
  • every time you make a subclass of one class, you also have to make a subclass of another
  • the prefixes of the class names in one hierarchy are the same as the prefixes in another hierarchy
  • Treatment
  • To make sure that instances of one hierarchy refer to instances of the other
    • Move Method
    • Move Field
      • the hierarchy on the referring class disappears

Dispensables (不用品の臭い）

Comments

  • Signs and Symptoms
  • comments often are used as a deodorant
    • you look at thickly commented code and notice that the comments are there, because the code is bad
  • Treatment
  • If you need a comment to explain what a block of code
    • Extract Method
  • you still need a comment to explain what it does
    • use Rename Method
  • If you need to state some rules about the required state of the system
    • use Introduce Assertion
  • Note
  • When you feel the need to write a comment, first try to refactor the code so that any comment becomes superfluous.
  • A comment is a good place to say WHY you did something.

Duplicated Code

  • if you see the same structure in more than one place
  • same expression in two methods of the same class
  • two sibling subclasses (brother)
  • Template Method
  • Substitute Algorithm
  • Extract Surrounding Method
  • two unrelated class
  • Extract Class
  • Extract Module
  • Where should be the duplicated Method located ?

Lazy Class

  • Signs and Symptoms
  • A class that isn't doing enough to pay for itself should be eliminated
  • Treatment
  • You let the class die
  • if it's a subclass or modules
    • Collapse Hierarchy
  • Useless components
    • should be subjected to Inline Class or Inline Module

Data Class

  • Signs and Symptoms
  • have attributes and nothing else
  • no responsibility class
  • Treatment
  • Use Remove Setting Method on any instance variable that should not be changed.
  • if you have collection instance variables
    • check Encapsulation
  • SRP
    • Try to use Move Method, Extract Method, Hide Method to give its nature responsibility of the role

Speculative Generality => YAGNI

  • Signs and Symptoms
  • Codes that isn't used, it's designed for future.
    • "oh, I think we need the ability to do this kind of thing someday"
    • all sorts of hooks and special cases to handle things that aren't required
  • Treatment
  • if the code aren't doing much
    • use Collapse Hierarchy
  • Unnecessary delegation
    • can be removed with Inline Class
  • Method with unused parameters
    • should be subject to Remove Parameter
  • methods name with odd names
    • should be brought down to earth with Rename Method.

Couplers (結合系）

Feature Envy

  • Signs and Symptoms
  • a method that seems more interested in a class other than the one it actually is in
  • Treatment
  • Move Method
  • Extract Method
  • The heuristic is to determine which class has most of the data and put the method with that data
  • Strategy Pattern
  • Visitor Pattern
  • Self-Delegation pattern
  • Idea The fundamental rule of thumb is to put things together that change together. Data the behavior that references that data usually change together, but there are exceptions.

Inappropriate Intimacy

  • Signs and Symptoms
  • One class uses the internal fields and methods of another class.
  • Treatment
  • Move Method and Move Field to separate the pieces to reduce the intimacy. See whether you can arrange a Change Bidrectionl Association to Undirectional.
  • If the classes do have common interests,
    • use Extract Class to put the commonality
    • use Hide Delegate to let another class act as go-between
  • Inheritance often can lead to over-intimacy
    • Replace Inheritance with Delegation(if it's needed)

Message Chains

  • Signs and Symptoms
  • Violation of LoD
  • Treatment
  • Hide Delegate
  • alternative way (less chain)
    • See what the resulting objet is used for. See whether you can use Extract Method to take a piece of the code that uses it and then move Method to push it down the chain.

Middle Man

  • Signs and Symptoms
  • If half or more of the methods are delegating to other class.Why does it exist at all?
  • Treatment
  • Remove Middle Man
    • Encapsulation, Delegation
  • When To Ignore
  • a middle man may have been added to avoid interclass dependencies.
  • some design patterns create middle man on purpose(Proxy, Decorator)

Other Smells (その他の臭い）

Incomplete Library Class

  • Signs and Symptoms
  • Library doesn't provide feature that we need
  • Treatment
  • Use Ruby's open classes
    • Move Method to move the behavior needed directly to the library class
  • Other Language
    • Introduce Foreign Method (few change)
    • Introduce Local Extension (big change)

Metaprogramming Madness

  • Signs and Symptoms
  • Treatment
  • if it's not necessary
    • Replace Dynamic Receptor with Dynamic Method Definition
    • Extract Method
  • if it's truly needed
    • Isolate Dynamic Receptor to separate concerns

Disjointed API (支離滅裂)

  • Signs and Symptoms
  • with many configuration options
  • The same configuration options will be used over and over
  • Treatment
  • Introduce Gateway
  • Introduce Expression Builder

Repetitive Boilerplate

  • Signs and Symptoms
  • method called from multiple places
  • Treatment
  • Class Annotation

続き

Refactoring Ruby Edition ７タイプの実践編

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
1
1