はじめに
リファクタリングとは、外部の振る舞いを保ったままで、
内部構造を改善していく作業のことです。
リファクタリングすると、以前書いたコードの向上が期待されます。
セオリーでいくと、設計したあとに実装するのが一般的ですが、
長期間にわたってコードが修正され、
設計に基づいたシステムの統一性や構造は次第に崩れいていきます。
リファクタリングによる間違った設計やカオス状態のコードを、
優れた設計のコードに再構築できます。
こちらの本を参考に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パターンが使われていることは意識しません。
まとめ
如何だったでしょうか。
下記の観点でリファクタリングしました。
①処理をメソッドに分割する
②メソッドの責務に応じて、別のクラスへ移動する
③条件文をポリモーフィズムに置き換える
これらは、いずれも責務を分割し、
保守しやすいコードを書くのに役に立ちます。
初級者にとって、設計を学習する際に、
最初から完璧な設計は難しいと思います。
リファクタリングを繰り返しながら、
理想の設計を探るのも良いかもしれませんね。
以上です。