[Swift] サンプルを見ながら、リファクタリングを勉強する

はじめに

リファクタリングとは、外部の振る舞いを保ったままで、
内部構造を改善していく作業のことです。

リファクタリングすると、以前書いたコードの向上が期待されます。

セオリーでいくと、設計したあとに実装するのが一般的ですが、
長期間にわたってコードが修正され、
設計に基づいたシステムの統一性や構造は次第に崩れいていきます。

リファクタリングによる間違った設計やカオス状態のコードを、
優れた設計のコードに再構築できます。

こちらの本を参考にSwift版で纏めてみました。

リファクタリングについて学ぶ

リファクタリング対象のコードの例

ビデオレンタルの料金を計算して
計算書を印刷するプログラムを例に説明します。

システムには、どの映画を何日間借りる借りるかが入力されます。
すると貸出日数によって料金が計算され、映画の分類が判定されます。

映画の分類は、3種類あり、一般向け、子供向け、新作と分類されます。
計算書には、料金の他にレンタルポイントも印刷します。

Moveクラス

映画データ保持用のクラス

Movie.swift

import UIKit

enum PriceCodeType: Int {
    case regular
    case newRelease
    case childrens
}

final class Movie: NSObject {

    private var title = ""
    private var priceCode = PriceCodeType.regular

    init(title: String, priceCode: PriceCodeType) {
        super.init()
        self.title = title
        self.priceCode = priceCode
    }

    func getPriceCode() -> PriceCodeType {
        return priceCode
    }

    func setPriceCode(priceCode: PriceCodeType) {
        self.priceCode = priceCode
    }

    func getTitle() -> String {
        return title
    }
}

Rentalクラス

顧客がビデオを借りたことを管理するクラス

Rental.swift

import UIKit

final class Rental: NSObject {

    private var movie: Movie?
    private var dayRented = 0

    init(movie: Movie, dayRented: Int) {
        super.init()
        self.movie = movie
        self.dayRented = dayRented
    }

    func getDayRented() -> Int {
        return dayRented
    }

    func getMovie() -> Movie {
        return movie!
    }
}

Customerクラス

取り扱う顧客を管理するクラス

Customer.swift

import UIKit

final class Customer: NSObject {

    private var name = ""
    private var rentals:[Rental] = []

    init(name: String) {
        super.init()
        self.name = name
    }

    func addRental(rental: Rental) {
        rentals.append(rental)
    }

    func getName() -> String {
        return name
    }

    func statement() -> String {

        var totalAmount = Double(0)
        var frequentRenterPoints = 0
        var result = "Rental Record for \(getName()) \n"

        //一行毎に金額を計算
        for rental in rentals {

            var thisAmount = Double(0)

            switch rental.getMovie().getPriceCode() {
            case .regular:
                thisAmount += 2

                if rental.getDayRented() > 2 {
                    thisAmount += Double(rental.getDayRented() - 2) * 1.5
                }
            case .newRelease:
                thisAmount += Double(rental.getDayRented() * 3)

            case .childrens:
                thisAmount += 1.5
                if rental.getDayRented() > 3 {
                    thisAmount += Double(rental.getDayRented() - 3) * 1.5
                }
            }

            //レンタルポイントを加算
            frequentRenterPoints += 1

            if rental.getMovie().getPriceCode() == PriceCodeType.newRelease &&
                rental.getDayRented() > 1 {
                frequentRenterPoints += 1
            }

            //この貸出に関する数値の表示
            result += "\t \(rental.getMovie().getTitle()) \t \(thisAmount) \n"
            totalAmount += thisAmount
        }

        //フッタ部分の追加
        result += "Amount owed is \(totalAmount) \n"
        result += "You earned \(frequentRenterPoints) frequent renter point"
        return result
    }
}

何が問題か？

これでも動きますが、可読性が悪く、変更が容易ではありません。
例えば、料金計算の変更がポイント計算に影響を与えてしまいます。
さらに、ルールが複雑になると、どこを修正すべきかわかりづらくなります。

よって、プログラムの変更を少なくし、影響範囲を限定的にする工夫は必要です。

どうすべきか？

statementメソッドの分割、再配置をしながら、リファクタリングを進めていきます。

[観点]
①処理をメソッドに分割する
②メソッドの責務に応じて、別のクラスへ移動する
③条件文をポリモーフィズムに置き換える

①処理をメソッドに分割する

長いメソッドは、もっと短く分割できないかを考えます。
まずは、「金額を計算する」部分をメソッドに分割します。

メソッド名は、「amountFor」です。

Customer.swift

import UIKit

final class Customer: NSObject {

    private var name = ""
    private var rentals:[Rental] = []

    init(name: String) {
        super.init()
        self.name = name
    }

    func addRental(rental: Rental) {
        rentals.append(rental)
    }

    func getName() -> String {
        return name
    }

    func statement() -> String {

        var totalAmount = Double(0)
        var frequentRenterPoints = 0
        var result = "Rental Record for \(getName()) \n"

        //一行毎に金額を計算
        for rental in rentals {

            let thisAmount = amountFor(rental: rental)

            //レンタルポイントを加算
            frequentRenterPoints += 1

            if rental.getMovie().getPriceCode() == PriceCodeType.newRelease &&
                rental.getDayRented() > 1 {
                frequentRenterPoints += 1
            }

            //この貸出に関する数値の表示
            result += "\t \(rental.getMovie().getTitle()) \t \(thisAmount) \n"
            totalAmount += thisAmount
        }

        //フッタ部分の追加
        result += "Amount owed is \(totalAmount) \n"
        result += "You earned \(frequentRenterPoints) frequent renter point"
        return result
    }

    private func amountFor(rental: Rental) -> Double {

        var result = Double(0)

        switch rental.getMovie().getPriceCode() {
        case .regular:
            result += 2

            if rental.getDayRented() > 2 {
                result += Double(rental.getDayRented() - 2) * 1.5
            }
        case .newRelease:
            result += Double(rental.getDayRented() * 3)

        case .childrens:
            result += 1.5
            if rental.getDayRented() > 3 {
                result += Double(rental.getDayRented() - 3) * 1.5
            }
        }
        return result
    }

②メソッドの責務に応じて、別のクラスへ移動する

②-1 「金額を計算する部分」を移動する

amountForをメソッドに分割しましたが、
Rentalクラスの情報は使っているものの、Customerクラスの情報を使っていないため、
Customerクラスに本メソッドがあるのは不自然です。

CustomerクラスのamountForメソッドをRentalクラスに移動します。
移動するに伴い、少しリファクタリングします。

Retanl.swift

import UIKit

final class Rental: NSObject {

    private var movie: Movie?
    private var dayRented = 0

    init(movie: Movie, dayRented: Int) {
        super.init()
        self.movie = movie
        self.dayRented = dayRented
    }

    func getDayRented() -> Int {
        return dayRented
    }

    func getMovie() -> Movie {
        return movie!
    }

    func getCharge() -> Double {

        var result = Double(0)

        switch getMovie().getPriceCode() {
        case .regular:
            result += 2

            if getDayRented() > 2 {
                result += Double(getDayRented() - 2) * 1.5
            }
        case .newRelease:
            result += Double(getDayRented() * 3)

        case .childrens:
            result += 1.5
            if getDayRented() > 3 {
                result += Double(getDayRented() - 3) * 1.5
            }
        }
        return result
    }
}

Customer.swift

import UIKit

final class Customer: NSObject {

    private var name = ""
    private var rentals:[Rental] = []

    init(name: String) {
        super.init()
        self.name = name
    }

    func addRental(rental: Rental) {
        rentals.append(rental)
    }

    func getName() -> String {
        return name
    }

    func statement() -> String {

        var totalAmount = Double(0)
        var frequentRenterPoints = 0
        var result = "Rental Record for \(getName()) \n"

        //一行毎に金額を計算
        for rental in rentals {

            let thisAmount = amountFor(rental: rental)

            //レンタルポイントを加算
            frequentRenterPoints += 1

            if rental.getMovie().getPriceCode() == PriceCodeType.newRelease &&
                rental.getDayRented() > 1 {
                frequentRenterPoints += 1
            }

            //この貸出に関する数値の表示
            result += "\t \(rental.getMovie().getTitle()) \t \(thisAmount) \n"
            totalAmount += thisAmount
        }

        //フッタ部分の追加
        result += "Amount owed is \(totalAmount) \n"
        result += "You earned \(frequentRenterPoints) frequent renter point"
        return result
    }

    private func amountFor(rental: Rental) -> Double {

        //アクセスするインスタンスを変更しました。
        return rental.getCharge()
    }
}

thisAmountが冗長なので、さらにリファクタリングします。

Customer.swift

import UIKit

final class Customer: NSObject {

    private var name = ""
    private var rentals:[Rental] = []

    init(name: String) {
        super.init()
        self.name = name
    }

    func addRental(rental: Rental) {
        rentals.append(rental)
    }

    func getName() -> String {
        return name
    }

    func statement() -> String {

        var totalAmount = Double(0)
        var frequentRenterPoints = 0
        var result = "Rental Record for \(getName()) \n"

        //一行毎に金額を計算
        for rental in rentals {

            //レンタルポイントを加算
            frequentRenterPoints += 1

            if rental.getMovie().getPriceCode() == PriceCodeType.newRelease &&
                rental.getDayRented() > 1 {
                frequentRenterPoints += 1
            }

            //この貸出に関する数値の表示
            result += "\t \(rental.getMovie().getTitle()) \t \(rental.getCharge()) \n"
            totalAmount += rental.getCharge()
        }

        //フッタ部分の追加
        result += "Amount owed is \(totalAmount) \n"
        result += "You earned \(frequentRenterPoints) frequent renter point"
        return result
    }
}

②-2 「レンタルポイントの計算」を移動する

「レンタルポイントの計算」部分もRentalクラスに責務を任せて良さそうです。
CustomerクラスのgetFrequentRenterPointメソッドをRenalクラスに移動します。

Rental

import UIKit

final class Rental: NSObject {

    private var movie: Movie?
    private var dayRented = 0

    init(movie: Movie, dayRented: Int) {
        super.init()
        self.movie = movie
        self.dayRented = dayRented
    }

    func getDayRented() -> Int {
        return dayRented
    }

    func getMovie() -> Movie {
        return movie!
    }

    func getCharge() -> Double {

        var result = Double(0)

        switch getMovie().getPriceCode() {
        case .regular:
            result += 2

            if getDayRented() > 2 {
                result += Double(getDayRented() - 2) * 1.5
            }
        case .newRelease:
            result += Double(getDayRented() * 3)

        case .childrens:
            result += 1.5
            if getDayRented() > 3 {
                result += Double(getDayRented() - 3) * 1.5
            }
        }
        return result
    }

    func getFrequentRenterPoints() -> Int {

        if getMovie().getPriceCode() == PriceCodeType.newRelease &&
            getDayRented() > 1 {
            return 2
        } else {
            return 1
        }
    }
}

Customer.swift

import UIKit

final class Customer: NSObject {

    private var name = ""
    private var rentals:[Rental] = []

    init(name: String) {
        super.init()
        self.name = name
    }

    func addRental(rental: Rental) {
        rentals.append(rental)
    }

    func getName() -> String {
        return name
    }

    func statement() -> String {

        var totalAmount = Double(0)
        var frequentRenterPoints = 0
        var result = "Rental Record for \(getName()) \n"

        //一行毎に金額を計算
        for rental in rentals {

            //レンタルポイントを加算
            frequentRenterPoints += rental.getFrequentRenterPoints()

            //この貸出に関する数値の表示
            result += "\t \(rental.getMovie().getTitle()) \t \(rental.getCharge()) \n"
            totalAmount += rental.getCharge()
        }

        //フッタ部分の追加
        result += "Amount owed is \(totalAmount) \n"
        result += "You earned \(frequentRenterPoints) frequent renter point"
        return result
    }
}

③条件文をポリモーフィズムに置き換える

switch文で他のオブジェクトを調べるのは、適切ではありません。
switch文は、常に自分自身について、行うべきです。

③-1 RentalクラスのgetMovieは、Movieクラスへ移動します。

Movie.swift

import UIKit

enum PriceCodeType: Int {
    case regular
    case newRelease
    case childrens
}

final class Movie: NSObject {

    private var title = ""
    private var priceCode = PriceCodeType.regular

    init(title: String, priceCode: PriceCodeType) {
        super.init()
        self.title = title
        self.priceCode = priceCode
    }

    func getPriceCode() -> PriceCodeType {
        return priceCode
    }

    func setPriceCode(priceCode: PriceCodeType) {
        self.priceCode = priceCode
    }

    func getTitle() -> String {
        return title
    }

    func getCharge(dayRented: Int) -> Double {

        var result = Double(0)

        switch getPriceCode() {
        case .regular:
            result += 2

            if dayRented > 2 {
                result += Double(dayRented - 2) * 1.5
            }
        case .newRelease:
            result += Double(dayRented * 3)

        case .childrens:
            result += 1.5
            if dayRented > 3 {
                result += Double(dayRented - 3) * 1.5
            }
        }
        return result
    }
}

Rental.swift

import UIKit

final class Rental: NSObject {

    private var movie: Movie?
    private var dayRented = 0

    init(movie: Movie, dayRented: Int) {
        super.init()
        self.movie = movie
        self.dayRented = dayRented
    }

    func getDayRented() -> Int {
        return dayRented
    }

    func getMovie() -> Movie {
        return movie!
    }

    func getFrequentRenterPoints() -> Int {

        if getMovie().getPriceCode() == PriceCodeType.newRelease &&
            getDayRented() > 1 {
            return 2
        } else {
            return 1
        }
    }

    func getCharge() -> Double {
        return (movie?.getCharge(dayRented: dayRented))!
    }
}

④-2 レンタルポイントの計算についても、同様に行います。

Movie.swift

import UIKit

enum PriceCodeType: Int {
    case regular
    case newRelease
    case childrens
}

final class Movie: NSObject {

    private var title = ""
    private var priceCode = PriceCodeType.regular

    init(title: String, priceCode: PriceCodeType) {
        super.init()
        self.title = title
        self.priceCode = priceCode
    }

    func getPriceCode() -> PriceCodeType {
        return priceCode
    }

    func setPriceCode(priceCode: PriceCodeType) {
        self.priceCode = priceCode
    }

    func getTitle() -> String {
        return title
    }

    func getCharge(dayRented: Int) -> Double {

        var result = Double(0)

        switch getPriceCode() {
        case .regular:
            result += 2

            if dayRented > 2 {
                result += Double(dayRented - 2) * 1.5
            }
        case .newRelease:
            result += Double(dayRented * 3)

        case .childrens:
            result += 1.5
            if dayRented > 3 {
                result += Double(dayRented - 3) * 1.5
            }
        }
        return result
    }

    func getFrequentRenterPoints(dayRented: Int) -> Int {

        if getPriceCode() == PriceCodeType.newRelease &&
            dayRented > 1 {
            return 2
        } else {
            return 1
        }
    }
}

Rental.swift

import UIKit

final class Rental: NSObject {

    private var movie: Movie?
    private var dayRented = 0

    init(movie: Movie, dayRented: Int) {
        super.init()
        self.movie = movie
        self.dayRented = dayRented
    }

    func getDayRented() -> Int {
        return dayRented
    }

    func getMovie() -> Movie {
        return movie!
    }

    func getFrequentRenterPoints() -> Int {
        return (movie?.getFrequentRenterPoints(dayRented: dayRented))!
    }

    func getCharge() -> Double {
        return (movie?.getCharge(dayRented: dayRented))!
    }
}

④-3 プロトコルを定義します。

映画の分類がいくつかあって、それぞれが同じ質問に対して異なる答え方をしています。
これは、サブクラスの出番です。
映画のサブクラスを3つ用意すれば、各々が料金計算メソッドの別の実装を持つようにできます。

Movie.swift

protocol MovieType {
    func getPriceCode() -> PriceCodeType
    func getCharge(dayRented: Int) -> Double
    func getFrequentRenterPoints(dayRented: Int) -> Int
}

④-4 サブクラスを定義します。

RegularMovie.swift

import UIKit

final class RegularMovie: MovieType {

    func getPriceCode() -> PriceCodeType {
        return PriceCodeType.regular
    }

    func getCharge(dayRented: Int) -> Double {

        var result = Double(2)

        if dayRented > 2 {
            result += Double(dayRented - 2) * 1.5
        }
        return result
    }

    func getFrequentRenterPoints(dayRented: Int) -> Int {
        return 1
    }
}

ChildrenMovie.swift

import UIKit

final class ChildrenMovie: MovieType {

    func getPriceCode() -> PriceCodeType {
        return PriceCodeType.childrens
    }

    func getCharge(dayRented: Int) -> Double {

        var result = Double(1.5)

        if dayRented > 3 {
            result += Double(dayRented - 3) * 1.5
        }
        return result
    }

    func getFrequentRenterPoints(dayRented: Int) -> Int {
        return 1
    }
}

NewReleaseMovie.swift

import UIKit

final class NewReleaseMovie: MovieType {

    func getPriceCode() -> PriceCodeType {
        return PriceCodeType.newRelease
    }

    func getCharge(dayRented: Int) -> Double {
        return Double(dayRented * 3)
    }

    func getFrequentRenterPoints(dayRented: Int) -> Int {
        return dayRented > 1 ? 2 : 1
    }
}

Movie.swift

final class Movie: NSObject {

    private var title = ""
    private var movieType: MovieType?

    init(title: String, priceCode: PriceCodeType) {
        super.init()
        self.title = title
        setPriceCode(priceCode: priceCode)
    }

    func getPriceCode() -> PriceCodeType {
        return (movieType?.getPriceCode())!
    }

    func setPriceCode(priceCode: PriceCodeType) {

        switch priceCode {
        case .regular:
            movieType = RegularMovie()

        case .newRelease:
            movieType = NewReleaseMovie()

        case .childrens:
            movieType = ChildrenMovie()
        }
    }

    func getTitle() -> String {
        return title
    }

    func getCharge(dayRented: Int) -> Double {
        return (movieType?.getCharge(dayRented: dayRented))!
    }

    func getFrequentRenterPoints(dayRented: Int) -> Int {
        return (movieType?.getFrequentRenterPoints(dayRented: dayRented))!
    }
}

Stateパターンの導入はかなり手間がかかりますが、
利点としては、料金体系が変わっても、Movieのサブクラスを追加し、
メソッドをオーバライドすればOKです。

こうすることにより、変更が非常に簡単になりました。
しかも、利用側は内部的にStateパターンが使われていることは意識しません。

まとめ

如何だったでしょうか。
下記の観点でリファクタリングしました。

①処理をメソッドに分割する
②メソッドの責務に応じて、別のクラスへ移動する
③条件文をポリモーフィズムに置き換える

これらは、いずれも責務を分割し、
保守しやすいコードを書くのに役に立ちます。

初級者にとって、設計を学習する際に、
最初から完璧な設計は難しいと思います。

リファクタリングを繰り返しながら、
理想の設計を探るのも良いかもしれませんね。

以上です。