リバーシリファクタリングチャレンジ
koherさんが公開された、このFat View Controller、あなたはリファクタリングできますか？チャレンジに参加しました。
本チャレンジは、 Fat View Controller として実装されたリバーシアプリをリファクタリングし、どれだけクリーンな設計とコードを実現できるかというコンペティションです（ジャッジが優劣を判定するわけではなく、設計の技を競い合うのが目的です）。
すばらしいチャレンジを用意くださったkoherさんを始め、運営のお手伝いをされているtakasekさん、Ogawaさんの皆様に感謝です。
リファクタリング結果
以下のGitHubリポジトリにリファクタリングした結果を公開しています。masterブランチがリファクタリング済みになります。
この記事にもコードを記載していますが全体の8割ぐらいです。
リファクタリング方針
ゼロから作り直すやり方ではなく、オリジナルのリバーシ部分のロジックを踏襲しつつ、少しずつ動作を確認しながらリファクタリングを実施しました。
iOSアプリ設計パターン入門にて、Reduxアーキテクチャの章を執筆したので、Reduxアーキテクチャを適用したリファクタリングにチャレンジします。ターン制のゲームはユーザーやコンピュータのアクションによりゲームの状態が変化し、状態の変化に合わせたビューの表示を行う流れは、Reduxアーキテクチャと相性が良いと感じました。
リファクタリング観点
以下のような観点を意識しながら、リファクタリングを行いました。
ただ、各観点を最大化することが目的ではなく、適切な範囲にとどめ全体最適を目指すことが大事だと思っています。
- 階層、分断、排他、網羅を意識した構造化
- 影響範囲の局所化
- 型表現や命名による意味付け
- 単一責務化
- 制約化
- 抽象化
- 共通化
アーキテクチャ導入
Reduxアーキテクチャ構造
リバーシアプリに適用したReduxアーキテクチャの構造は以下の図のようになります。
また、Reduxアーキテクチャを支援するReSwiftライブラリを導入しました。
Reduxは状態の変化を単一のフロー制御により制約付けられており、ViewからはActionをStoreにdispatchし、Reducer関数によってStateが変更されます。ViewはStoreから変更されたことが通知されるので、イミュータブルなStateにアクセスし前回のStateから変化した状態を割り出して適切なViewの表示を更新したり、Stateの状態を鑑みて新たなActionをStoreにdispatchしたりします。
レイヤ分割（Xcodeターゲット分割）
Reduxアーキテクチャを導入するにあたりターゲットを以下の2つに分割しました。
もっと規模が大きいアプリの場合は、LogicレイヤをCleanアーキテクチャを参考にレイヤ分割してもよいかもしれません。
- Reversiターゲット（ビューに関するコード）
- UIKit ViewController
- UIKit View
- Logicターゲット（ロジックに関するコード）
- Redux actions
- Redux store
- Redux state
- Redux reducer
依存関係は以下のとおりです。
- Reversiターゲット ⇒ Logicターゲット
Logicターゲットは以下のような目的で設けました。
- Viewのコードに依存しないビジネスロジックのコードを局所化するため
- UIKitに依存しないPureSwiftのコードを局所化するため
- ビジネスロジックのコードのViewに対する可視性を細かく制御（publicとinternalの使い分け）したかったため
可視性制御（publicとinternalの使い分け）
- stateやdataのstruct/classはロジック側でのみinitできるように制約する
- ⇒ View側で意図せず状態が生成され利用されることを抑止する
- internalなプロパティはテストコードからでも参照できるので、より詳細な内部の状態の確認に利用する
DataTypeの抽出
- 責務過多なデータ表現は用途を限定した単一責務のデータに分割する
- e.g.
DiskandSide
- e.g.
- 役割を担うプリミティブなデータ構造をデータ型と命名を与え意味付する
- e.g.
Coordinate,PlacedDiskCoordinate,OptionalDiskCoordinate
- e.g.
- 有限なデータを可変長[配列]で表現している箇所は有限で表現する
- ビューのインスタンスが保持しているデータをDataTypeとして抽出する
既存のDataType
public enum Player: Int, Equatable, Codable {
case manual = 0
case computer = 1
}
public enum Disk: String, Codable {
case diskDark
case diskLight
}
追加したDataType
public enum Side: String, Codable, CaseIterable {
case sideDark
case sideLight
}
extension Side: Hashable {}
extension Side {
public var index: Int {
switch self {
case .sideDark: return 0
case .sideLight: return 1
}
}
public var disk: Disk {
switch self {
case .sideDark: return .diskDark
case .sideLight: return .diskLight
}
}
}
extension Side {
var flipped: Side {
switch self {
case .sideDark: return .sideLight
case .sideLight: return .sideDark
}
}
}
public struct Coordinate: Equatable, Codable {
public var x: Int
public var y: Int
public init(x: Int, y: Int) {
self.x = x
self.y = y
}
}
infix operator +: AdditionPrecedence
extension Coordinate {
static func + (left: Coordinate, right: Coordinate) -> Coordinate {
return Coordinate(x: left.x + right.x, y: left.y + right.y)
}
}
public struct PlacedDiskCoordinate: Equatable, Codable {
public var disk: Disk
public var coordinate: Coordinate
public init(disk: Disk, coordinate: Coordinate) {
self.disk = disk
self.coordinate = coordinate
}
}
extension PlacedDiskCoordinate {
var optionalDiskCoordinate: OptionalDiskCoordinate { OptionalDiskCoordinate(disk: disk, coordinate: coordinate) }
}
public struct OptionalDiskCoordinate: Equatable, Codable {
public var disk: Disk?
public var coordinate: Coordinate
}
public struct BoardSetting: Equatable, Codable {
public var cols: Int
public var rows: Int
private var xRange: Range<Int> { 0 ..< self.cols }
private var yRange: Range<Int> { 0 ..< self.rows }
public var coordinates: [Coordinate] {
self.yRange.map { y in self.xRange.map { x in Coordinate(x: x, y: y) } }.flatMap { $0 }
}
public func validCoordinate(_ coordinate: Coordinate) -> Bool {
self.xRange.contains(coordinate.x) && self.yRange.contains(coordinate.y)
}
init(cols: Int, rows: Int) {
self.cols = cols
self.rows = rows
}
}
補足
データがEquatableに準拠しているのはView側でStateの変更通知を受け取り、Stateへアクセスしたときに前回の変更通知から、どこが変わったのかわからないため前回のStateと比較して差分を検知するために利用します。
GameProgress（ゲーム進行状態表現）
ゲームの進行状態を表現するGameProgressを導入しました。
- このゲームの進行状態は有限でかつ排他的なのでenumで表現
- 各進行状態下に付随する取りうる状態をassociated valueを活用して包括関係を表現
public enum GameProgress: Equatable {
case initialing
case turn(Progress, Side, Player, ComputerThinking)
case gameOver(GameOver)
case interrupt(Interrupt)
}
public enum Side: String, Codable, CaseIterable {
case sideDark
case sideLight
}
public enum ComputerThinking: String, Equatable, Codable {
case none
case thinking
}
public enum GameOver: Equatable {
case won(Side)
case tied
}
public enum Interrupt: Equatable {
case resetConfrmation(Alert)
case cannotPlaceDisk(Alert)
}
public enum Alert: String, Equatable, Codable {
case none
case shouldShow
case showing
}
public enum Progress: Equatable {
case start
case progressing
}
以下の表はGameProgressが取りうる主要な状態のパターンを表しています。
（注意：この表は便宜上のため、正しい包括関係と網羅性を担保できていません）
| GameProgress | Associated value1 | Associated value2 | Associated value3 |
|---|---|---|---|
| initialing | --- | --- | --- |
| turn | Side.dark | Player.manual | ComputerThinking.none |
| 〃 | 〃 | Player.computer | ComputerThinking.none |
| 〃 | 〃 | 〃 | ComputerThinking.thinking |
| 〃 | Side.light | Player.manual | ComputerThinking.none |
| 〃 | 〃 | Player.computer | ComputerThinking.none |
| 〃 | 〃 | 〃 | ComputerThinking.thinking |
| gameOver | GameOver.won | Side.dark | --- |
| 〃 | 〃 | Side.light | --- |
| 〃 | GameOver.tied | --- | --- |
| interrupt | resetConfirmation | Alert.none | --- |
| 〃 | 〃 | Alert.shouldShow | --- |
| 〃 | 〃 | Alert.showing | --- |
| 〃 | cannotPlaceDisk | Alert.none | --- |
| 〃 | 〃 | Alert.shouldShow | --- |
| 〃 | 〃 | Alert.showing | --- |
interrup（割り込み状態表現）
アラートを表示中の状態管理が悩ましかったのでinterrupt状態を導入しゲーム中への割り込みを表現しています。
既存コードでは、リセットのアラートを表示中でも双方のプレイヤーがコンピュータであればゲームは進行します。
このとき、プレイヤーが手詰まりになると、手詰まりのアラートを表示するのですが、2つのアラート表示がバッティングするので対処が必要になります。
本リファクタリングでは、リセットアラートを表示中はコンピュータによるゲームを進行しない方針としました。
interrupt状態はturn状態と排他的な状態で、ゲームの進行（コンピュータの思考）はturn状態のみ実行できるものとすることで実現しました。
また、interruptは手詰まりのアラートをシステムから表示する場合も割り込みとして表現しました。
| 既存コード | リファクタリングコード |
|---|---|
| リセットアラートが表示中でもゲームが進行する | リセットアラートが表示中だとゲームが進行しない |
 |
 |
Redux
AppStateとAcrtionのすべてのコードはGitHub repositoryで確認できます。
AppState
AppStateはViewから参照される状態の起点になるStateです。
GameProgressはAppStateの内部状態から現在あるべきGameProgressの状態をComputed propertyを用いて割り出しています。
import Foundation
import ReSwift
public struct AppState: StateType, Codable {
public var boardContainer: BoardContainer
public var playerDark: PlayerSide = .init(side: .sideDark)
public var playerLight: PlayerSide = .init(side: .sideLight)
public var gameProgress: GameProgress {
if isInitialing {
return .initialing
} else if cannotPlaceDiskAlert != .none {
return .interrupt(.cannotPlaceDisk(cannotPlaceDiskAlert))
} else if resetConfirmationAlert != .none {
return .interrupt(.resetConfirmation(resetConfirmationAlert))
} else if let side = side {
let progress: Progress = turnStart ? .start : .progressing
let player: Player
switch side {
case .sideDark: player = playerDark.player
case .sideLight: player = playerLight.player
}
return .turn(progress, side, player, computerThinking)
} else if let winnerSide = boardContainer.board.sideWithMoreDisks() {
return .gameOver(.won(winnerSide))
} else {
return .gameOver(.tied)
}
}
var id: String = NSUUID().uuidString // prevent override uing reseted state
var side: Side? = .sideDark
var turnStart: Bool = false
var isInitialing: Bool = true
var isLoadedGame: Bool = false // prevent duplicate load game calls
var computerThinking: ComputerThinking = .none
var cannotPlaceDiskAlert: Alert = .none
var resetConfirmationAlert: Alert = .none
init(boardSetting: BoardSetting = .init(cols: 8, rows: 8)) {
self.boardContainer = .init(diskCoordinatesState: Board(boardSetting: boardSetting))
}
}
Reducer
Reducerはミューテーション可能なStateのコピーを一時的に生成し、新たな状態を反映させたうえ、イミュータブルなStateとして返す純粋関数です。
各Actionによって、どのように状態が変化するのか一目瞭然となっています。
func reducer(action: Action, state: AppState?) -> AppState {
var state = state ?? .init()
if state.turnStart {
state.turnStart = false
}
if let action = action as? AppAction {
switch action {
case .startGame:
state.isInitialing = false
case .placeDisk(let placedDiskCoordinate):
let flippedDiskCoordinates = state.boardContainer.board.flippedDiskCoordinatesByPlacingDisk(placedDiskCoordinate)
guard !flippedDiskCoordinates.isEmpty else { return state }
let changed: BoardChanged = .init(placedDiskCoordinate: placedDiskCoordinate, flippedDiskCoordinates: flippedDiskCoordinates)
changed.changedDiskCoordinate.forEach { state.boardContainer.board[$0.coordinate] = $0.optionalDiskCoordinate }
state.boardContainer.changed = changed
state.playerDark.count = state.boardContainer.board.count(of: .diskDark)
state.playerLight.count = state.boardContainer.board.count(of: .diskLight)
case .cannotPlaceDisk(let alert):
state.cannotPlaceDiskAlert = alert
case .resetConfirmation(let alert):
state.resetConfirmationAlert = alert
}
}
if let action = action as? AppPrivateAction {
switch action {
case .nextTurn:
guard case .none = state.resetConfirmationAlert else { return state }
guard let temp = state.side else { assertionFailure(); return state }
state.cannotPlaceDiskAlert = .none
let side = temp.flipped
state.side = side
case .validateTurn:
guard let side = state.side else { return state }
if state.boardContainer.board.validMoves(for: side).isEmpty {
if state.boardContainer.board.validMoves(for: side.flipped).isEmpty {
state.side = nil // GameOver
} else {
state.cannotPlaceDiskAlert = .shouldShow
}
} else {
state.turnStart = true
}
case .changePlayer(let side, let player):
switch side {
case .sideDark: state.playerDark.player = player
case .sideLight: state.playerLight.player = player
}
state.turnStart = true
if side == state.side {
state.computerThinking = .none
}
case .resetAllState:
var newState = AppState()
newState.playerDark = .init(side: .sideDark, count: newState.boardContainer.board.count(of: .diskDark))
newState.playerLight = .init(side: .sideLight, count: newState.boardContainer.board.count(of: .diskLight))
return newState
case .finisedLoadGame(let loadedAppState):
return loadedAppState
case .finisedSaveGame:
break
case .startComputerThinking:
state.computerThinking = .thinking
case .endComputerThinking:
state.computerThinking = .none
}
}
return state
}
Action
Actionはenumで表現して、Reducerで処理を行うべき網羅性を担保しています。
PrivateActionは、Viewからdispatchしない、ActionCreatorからのみdisptachするActionとして設けています。
public enum AppAction: Action {
case startGame
case placeDisk(PlacedDiskCoordinate)
case cannotPlaceDisk(Alert)
case resetConfirmation(Alert)
}
enum AppPrivateAction: Action {
case nextTurn
case validateTurn
case changePlayer(side: Side, player: Player)
case resetAllState
case finisedLoadGame(AppState)
case startComputerThinking
case endComputerThinking
case finisedSaveGame
}
struct ErrorAction: Action {
let error: Error
let title: String
let message: String
}
ActionCreator
ActionCreatorはReducerのようにStateの変更を行いませんが、以下のようなロジックの処理を担っています。
- 副作用（ゲームデータの保存・読み込み）が発生する処理
- 条件によってdispatchしたいActionを変更したい処理
- 複数のActionをdispatchしたい処理
また、副作用を伴う処理に依存する部分は、テスタブルにするためDependency Injectionしています。
extension AppAction {
public static func newGame() -> Thunk<AppState> {
return Thunk<AppState> { dispatch, getState, dependency in
print("- Logic.AppAction.newGame() START")
dispatch(AppPrivateAction.resetAllState)
dispatch(AppAction.saveGame())
print("- Logic.AppAction.newGame() END")
}
}
public static func saveGame() -> Thunk<AppState> {
return Thunk<AppState> { dispatch, getState, dependency in
print("- Logic.AppAction.saveGame() START")
do {
guard var state = getState() else { preconditionFailure() }
state.isInitialing = true
state.boardContainer.changed = nil
state.computerThinking = .none
state.resetConfirmationAlert = .none
try dependency.persistentInteractor.saveGame(state)
dispatch(AppPrivateAction.finisedSaveGame)
} catch let error {
dump(error)
let title = "Error occurred."
let message = "Cannot save games."
dispatch(ErrorAction(error: error, title: title, message: message))
}
print("- Logic.AppAction.saveGame() END")
}
}
public static func loadGame() -> Thunk<AppState> {
return Thunk<AppState> { dispatch, getState, dependency in
print("- Logic.AppAction.loadGame() START")
do {
guard let state = getState() else { preconditionFailure() }
guard state.isLoadedGame == false else { return } // prevent duplicate load game calls
dispatch(AppPrivateAction.resetAllState)
let loadData = try dependency.persistentInteractor.loadGame()
dispatch(AppPrivateAction.finisedLoadGame(loadData))
dispatch(AppPrivateAction.validateTurn)
} catch let error {
dump(error)
dispatch(AppAction.newGame())
}
print("- Logic.AppAction.loadGame() END")
}
}
public static func nextTurn() -> Thunk<AppState> {
return Thunk<AppState> { dispatch, getState, dependency in
guard let state = getState() else { preconditionFailure() }
if case .turn(_, let side, _, _) = state.gameProgress {
print("- Logic.AppAction.nextTurn() from: \(side) to: \(side.flipped)")
}
dispatch(AppPrivateAction.nextTurn)
dispatch(AppPrivateAction.validateTurn)
}
}
public static func changePlayer(side: Side, player: Player) -> Thunk<AppState> {
return Thunk<AppState> { dispatch, getState, dependency in
print("- Logic.AppAction.changePlayer(side: \(side), player: \(player)) START")
dispatch(AppPrivateAction.changePlayer(side: side, player: player))
dispatch(AppAction.saveGame())
print("- Logic.AppAction.changePlayer(side: \(side), player: \(player)) END")
}
}
public static func waitForPlayer() -> Thunk<AppState> {
return Thunk<AppState> { dispatch, getState, dependency in
print("- Logic.AppAction.waitForPlayer() START")
guard let state = getState() else { preconditionFailure() }
switch state.gameProgress {
case .turn(_, _, let player, _):
switch player {
case .manual:
break
case .computer:
dispatch(AppAction.playTurnOfComputer())
}
case .initialing, .interrupt, .gameOver:
assertionFailure()
}
print("- Logic.AppAction.waitForPlayer() END")
}
}
private static func playTurnOfComputer() -> Thunk<AppState> {
return Thunk<AppState> { dispatch, getState, dependency in
print("- Logic.AppAction.playTurnOfComputer() START")
guard let state = getState() else { preconditionFailure() }
switch state.gameProgress {
case .turn(_, let side, _, _):
let candidates = state.boardContainer.board.validMoves(for: side)
switch candidates.isEmpty {
case true:
dispatch(AppAction.nextTurn())
case false:
guard let candidate = candidates.randomElement() else { preconditionFailure() }
let id = state.id
store.dispatch(AppPrivateAction.startComputerThinking)
DispatchQueue.main.asyncAfter(deadline: .now() + dependency.computerThinkingTime) {
guard let state = getState() else { preconditionFailure() }
guard id == state.id else { return } // maybe reset game
guard case .turn(_, let sideEnd, _, let computerThinkingEnd) = state.gameProgress else { return }
guard case .thinking = computerThinkingEnd, side == sideEnd else { return } // maybe chaned to manual player
guard case .none = state.resetConfirmationAlert else { return }
dispatch(AppPrivateAction.endComputerThinking)
dispatch(AppAction.placeDisk(candidate))
}
}
case .initialing, .interrupt, .gameOver:
assertionFailure()
}
print("- Logic.AppAction.playTurnOfComputer() END")
}
}
}
Dependency & Middleware
PersistentInteractor,RepositoryはDependency Injectionできようにすために、protocolで抽象化し、createThunkMiddleware経由でDIしています。
public let store = Store<AppState>(
reducer: reducer,
state: AppState(),
middleware: [thunkMiddleware, loggingMiddleware]
)
protocol Dependency {
var persistentInteractor: PersistentInteractor { get }
var computerThinkingTime: DispatchTimeInterval { get }
}
struct DependencyImpl: Dependency {
let persistentInteractor: PersistentInteractor
let computerThinkingTime: DispatchTimeInterval
init(persistentInteractor: PersistentInteractor = PersistentInteractorImpl(), computerThinkingTime: DispatchTimeInterval = DispatchTimeInterval.milliseconds(1000)) {
self.persistentInteractor = persistentInteractor
self.computerThinkingTime = computerThinkingTime
}
}
let thunkMiddleware: Middleware<AppState> = createThunkMiddleware()
public struct Thunk<State>: Action {
let body: (_ dispatch: @escaping DispatchFunction, _ getState: @escaping () -> State?, _ dependency: Dependency) -> Void
init(body: @escaping (
_ dispatch: @escaping DispatchFunction,
_ getState: @escaping () -> State?,
_ dependency: Dependency) -> Void) {
self.body = body
}
}
func createThunkMiddleware<State>(dependency: Dependency = DependencyImpl()) -> Middleware<State> {
return { dispatch, getState in
return { next in
return { action in
switch action {
case let thunk as Thunk<State>:
thunk.body(dispatch, getState, dependency)
default:
next(action)
}
}
}
}
}
let loggingMiddleware: Middleware<AppState> = { dispatch, getState in
return { next in
return { action in
dump(action)
if case AppPrivateAction.validateTurn = action {
print(getState()?.boardContainer.board.debugDescription ?? "N/A")
}
return next(action)
}
}
}
PersistentInteractor & Repository
ゲーム状態をファイルに保存・読み込みするためのロジックです。
既存では独自のファイルファーマット形式で保存していましたが、ReduxのStateをCodableに準拠してJSON形式でStateを丸ごと保存するようにしました。
メリット
- 独自のファイルファーマット形式のパースロジックを排除し、一般的でかつパースAPIが提供されているJSONを利用できた
デメリット
- Stateの構造を変更すると、下位互換がなくなってしまいデータをロードできなくなってしまった
結論
- 過剰なリファクタリングでした
protocol PersistentInteractor {
func saveGame(_ appState: AppState) throws /* PersistentError */
func loadGame() throws -> AppState /* PersistentError */
}
struct PersistentInteractorImpl: PersistentInteractor {
enum PersistentError: Error {
case write(cause: Error?)
case read(cause: Error?)
}
private let repository: Repository
init(repository: Repository = RepositoryImpl()) {
self.repository = repository
}
func encode(_ appState: AppState) throws -> Data {
let encoder = JSONEncoder()
encoder.outputFormatting = .prettyPrinted
return try encoder.encode(appState)
}
func saveGame(_ appState: AppState) throws {
do {
let data = try encode(appState)
try repository.saveData(data)
} catch let error {
throw PersistentError.read(cause: error)
}
}
func loadGame() throws -> AppState {
do {
let data = try repository.loadData()
return try JSONDecoder().decode(AppState.self, from: data)
} catch let error {
throw PersistentError.write(cause: error)
}
}
}
extension Coordinate { /* Codable */
enum CodingKeys: String, CodingKey {
case x
case y
}
public func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)
try container.encode(x, forKey: .x)
try container.encode(y, forKey: .y)
}
}
ファイルに保存するときのファイル名はコンストラクタインジェクションできるようにし、テスト時は変更できるようにしました。
アプリ実行時とテスト実行時で保存したファイルを競合しないようにするためです。
protocol Repository {
func saveData(_ data: Data) throws /* FileIOError */
func loadData() throws -> Data /* FileIOError */
func clear() throws /* FileIOError */
}
struct RepositoryImpl: Repository {
enum FileIOError: Error {
case write(cause: Error?)
case read(cause: Error?)
case clear(cause: Error?)
}
let fileName: String
init(fileName: String = "appstate.json") {
self.fileName = fileName
}
private func createFileURL() throws -> URL {
try FileManager.default
.url(for: .applicationSupportDirectory, in: .userDomainMask, appropriateFor: nil, create: true)
.appendingPathComponent(fileName)
}
func saveData(_ data: Data) throws {
do {
let fileURL = try createFileURL()
try data.write(to: fileURL, options: [])
} catch let error {
throw FileIOError.read(cause: error)
}
}
func loadData() throws -> Data {
do {
let fileURL = try createFileURL()
return try Data(contentsOf: fileURL)
} catch let error {
throw FileIOError.write(cause: error)
}
}
func clear() throws {
do {
let fileURL = try createFileURL()
try FileManager.default.removeItem(at: fileURL)
} catch let error {
throw FileIOError.clear(cause: error)
}
}
}
ViewController
ViewControllerのすべてのコードはGitHub repositoryで確認できます。
ViewControllerのイニシャライザを利用してReduxのStreをコンストラクタインジェクションしたかっったので、storyboardからxibに変更しました。
class ViewController: UIViewController, StoreSubscriber {
...
private let store: Store<AppState>
init(store: Store<AppState> = Logic.store) {
self.store = store
super.init(nibName: nil, bundle: nil)
}
ViewControllerのコードは3種類の役割に分割できます。
- #1 / State handling（State -> Game management or View update）
- #2 / Game management（Game management -> Dispatch Action)
- #3 / View update
#1 / State handling（State -> Game management or View update）
Reduxから変更があった場合に通知を受、新たなStateの状態に基づいて、Game managementを行ったたり、Viewの表示を更新する指示を出しています。
-
func newState(state: AppState)はどんな変更であろうと通知され、表示を変更する処理を記載 -
subscriberGameProgressはGameProgressに変更があった場合のみ通知され、GameProgressの状況をパターンマッチのうえ、各状況における適切な処理を記載 -
subscriberBoardContainerはBoardContainerに変更があった場合のみ通知され、盤面の変更があった場合に盤面の表示を変更する処理を記載
class ViewController: UIViewController, StoreSubscriber {
...
override func viewDidLoad() {
super.viewDidLoad()
boardView.delegate = self
boardView.setUp(boardSetting: store.state.boardContainer.boardSetting)
messageDiskSize = messageDiskSizeConstraint.constant
store.subscribe(self)
store.subscribe(subscriberGameProgress) { appState in appState.select { $0.gameProgress }.skipRepeats() }
store.subscribe(subscriberBoardContainer) { appState in appState.select { $0.boardContainer }.skipRepeats() }
}
override func viewDidAppear(_ animated: Bool) {
super.viewDidAppear(animated)
loadGame()
}
func newState(state: AppState) {
updatePlayerControls(state.gameProgress, playerSide: state.playerDark)
updatePlayerControls(state.gameProgress, playerSide: state.playerLight)
updateCountLabels(state.playerDark)
updateCountLabels(state.playerLight)
updateMessageViews(state.gameProgress)
}
private lazy var subscriberGameProgress = BlockSubscriber<GameProgress>() { [unowned self] in
switch $0 {
case .initialing:
self.animationState.cancelAll()
self.startGame()
case .turn(let progress, let side, _, let computerThinking):
self.updatePlayerActivityIndicators(side: side, computerThinking: computerThinking)
switch progress {
case .start:
self.waitForPlayer()
case .progressing:
break
}
case .gameOver:
break
case .interrupt(let interrupt):
switch interrupt {
case .cannotPlaceDisk(let alert):
switch alert {
case .shouldShow:
self.showCannotPlaceDiskAlert()
case .none, .showing:
break
}
case .resetConfirmation(let alert):
switch alert {
case .shouldShow:
self.showRestConfirmationAlert()
case .none, .showing:
break
}
}
}
}
private lazy var subscriberBoardContainer = BlockSubscriber<BoardContainer>() { [unowned self] in
switch $0.changed {
case .none:
self.updateDisksForInitial($0.diskCoordinates)
case .some(let changed):
self.updateDisks(changed: changed, animated: true) { [weak self] _ in
self?.nextTurn()
}
}
}
}
Additional for ReSwift's subscriber
class BlockSubscriber<S>: StoreSubscriber {
typealias StoreSubscriberStateType = S
private let block: (S) -> Void
init(_ block: @escaping (S) -> Void) {
self.block = block
}
func newState(state: S) {
self.block(state)
}
}
#2 / Game management（Views -> State）
Game managementに関するメソッドが並びます。
View側からReduxのStateを変更するためのActionをdispatchする処理になります。
extension ViewController {
func saveGame() {
store.dispatch(AppAction.saveGame())
}
func loadGame() {
store.dispatch(AppAction.loadGame())
}
func newGame() {
animationState.cancelAll()
store.dispatch(AppAction.newGame())
}
func startGame() {
store.dispatch(AppAction.startGame)
}
func nextTurn() {
store.dispatch(AppAction.nextTurn())
}
func waitForPlayer() {
store.dispatch(AppAction.waitForPlayer())
}
func placeDisk(_ placedDiskCoordinate: PlacedDiskCoordinate) {
store.dispatch(AppAction.placeDisk(placedDiskCoordinate))
}
func changePlayer(side: Side, player: Player) {
store.dispatch(AppAction.changePlayer(side: side, player: player))
animationState.cancel(at: side)
}
func cannotPlaceDisk(alert: Alert) {
store.dispatch(AppAction.cannotPlaceDisk(alert))
}
func resetConfirmation(alert: Alert) {
store.dispatch(AppAction.resetConfirmation(alert))
}
}
#3 / View update（State -> Views）
Viewの表示を更新するためのメソッドが並びます。
extension ViewController {
func updateMessageViews(_ gameProgress: GameProgress) {
switch gameProgress {
case .initialing, .interrupt:
break
case .turn(_, let side, _, _):
messageDiskSizeConstraint.constant = messageDiskSize
messageDiskView.disk = side.disk
messageLabel.text = "'s turn"
case .gameOver(let gameOver):
switch gameOver {
case .won(let winner):
messageDiskSizeConstraint.constant = messageDiskSize
messageDiskView.disk = winner.disk
messageLabel.text = " won"
case .tied:
messageDiskSizeConstraint.constant = 0
messageLabel.text = "Tied"
}
}
}
func updateDisksForInitial(_ diskCoordinates: [OptionalDiskCoordinate]) {
diskCoordinates.forEach {
boardView.updateDisk($0.disk, coordinate: $0.coordinate, animated: false)
}
}
func updateDisks(changed: BoardChanged, animated isAnimated: Bool, completion: ((Bool) -> Void)? = nil) {
let disk = changed.placedDiskCoordinate.disk
let placedCoordinate = changed.placedDiskCoordinate.coordinate
let flippedCoordinates = changed.flippedDiskCoordinates.map { $0.coordinate }
if isAnimated {
animationState.createAnimationCanceller()
updateDisksWithAnimation(at: [placedCoordinate] + flippedCoordinates, to: disk) { [weak self] finished in
guard let self = self else { return }
if self.animationState.isCancelled { return }
self.animationState.cancel()
completion?(finished)
self.saveGame()
}
} else {
DispatchQueue.main.async { [weak self] in
guard let self = self else { return }
self.boardView.updateDisk(disk, coordinate: placedCoordinate, animated: false)
flippedCoordinates.forEach {
self.boardView.updateDisk(disk, coordinate: $0, animated: false)
}
completion?(true)
self.saveGame()
}
}
}
private func updateDisksWithAnimation<C: Collection>(at coordinates: C, to disk: Disk, completion: @escaping (Bool) -> Void)
where C.Element == Coordinate
{
guard let coordinate = coordinates.first else {
completion(true)
return
}
boardView.updateDisk(disk, coordinate: coordinate, animated: true) { [weak self] finished in
guard let self = self else { return }
if self.animationState.isCancelled { return }
if finished {
self.updateDisksWithAnimation(at: coordinates.dropFirst(), to: disk, completion: completion)
} else {
coordinates.forEach {
self.boardView.updateDisk(disk, coordinate: $0, animated: false)
}
completion(false)
}
}
}
private func updatePlayerActivityIndicators(side: Side, computerThinking: ComputerThinking) {
switch computerThinking {
case .thinking:
self.playerActivityIndicators[side.index].startAnimating()
case .none:
self.playerActivityIndicators.forEach { $0.stopAnimating() }
}
}
func updatePlayerControls(_ gameProgress: GameProgress, playerSide: PlayerSide) {
playerControls[playerSide.side.index].selectedSegmentIndex = playerSide.player.rawValue
playerControls.forEach {
switch gameProgress {
case .turn:
$0.isEnabled = true
case .initialing, .interrupt, .gameOver:
$0.isEnabled = false
}
}
}
func updateCountLabels(_ playerSide: PlayerSide) {
countLabels[playerSide.side.index].text = "\(playerSide.count)"
}
func showCannotPlaceDiskAlert() {
cannotPlaceDisk(alert: .showing)
let alertController = UIAlertController(
title: "Pass",
message: "Cannot place a disk.",
preferredStyle: .alert
)
alertController.addAction(UIAlertAction(title: "Dismiss", style: .default) { [weak self] _ in
self?.cannotPlaceDisk(alert: .none)
self?.nextTurn()
})
present(alertController, animated: true)
}
func showRestConfirmationAlert() {
resetConfirmation(alert: .showing)
let alertController = UIAlertController(
title: "Confirmation",
message: "Do you really want to reset the game?",
preferredStyle: .alert
)
alertController.addAction(UIAlertAction(title: "Cancel", style: .cancel) { [weak self] _ in
self?.resetConfirmation(alert: .none)
self?.waitForPlayer()
})
alertController.addAction(UIAlertAction(title: "OK", style: .default) { [weak self] _ in
self?.newGame()
})
present(alertController, animated: true)
}
}
User inputs
extension ViewController {
@IBAction func pressResetButton(_ sender: UIButton) {
resetConfirmation(alert: .shouldShow)
}
@IBAction func changePlayerControlSegment(_ sender: UISegmentedControl) {
guard let index = playerControls.firstIndex(of: sender) else { return }
let side: Side
switch index {
case 0: side = .sideDark
case 1: side = .sideLight
default: preconditionFailure()
}
changePlayer(side: side, player: sender.convertToPlayer)
}
}
extension ViewController: BoardViewDelegate {
func boardView(_ boardView: BoardView, didSelectCellAt coordinate: Coordinate) {
guard !animationState.isAnimating else { return }
guard case .turn(_, let side, let player, _) = store.state.gameProgress else { return }
guard case .manual = player else { return }
placeDisk(PlacedDiskCoordinate(disk: side.disk, coordinate: coordinate))
}
}
ロギング
リファクタリングの手がかりとして状態の変化を都度ログに出力しました。
- Action, ActionCreatorがdispatchされたときはパラメータも合わせて出力
- 次のターンになると盤面のデータ状態を出力
- Logic.AppAction.changePlayer(side: sideLight, player: computer) START
▿ Logic.AppPrivateAction.changePlayer
▿ changePlayer: (2 elements)
- side: Logic.Side.sideLight
- player: Logic.Player.computer
- Logic.AppAction.waitForPlayer() START
- Logic.AppAction.playTurnOfComputer() START
- Logic.AppPrivateAction.startComputerThinking
- Logic.AppAction.playTurnOfComputer() END
- Logic.AppAction.waitForPlayer() END
- Logic.AppAction.saveGame() START
- Logic.AppPrivateAction.finisedSaveGame
- Logic.AppAction.saveGame() END
- Logic.AppAction.changePlayer(side: sideLight, player: computer) END
- Logic.AppAction.nextTurn() from: sideDark to: sideLight
- Logic.AppPrivateAction.nextTurn
- Logic.AppPrivateAction.validateTurn
@01234567
0--------
1--------
2---x----
3---xx---
4---xo---
5--------
6--------
7--------
- Logic.AppAction.waitForPlayer() START
- Logic.AppAction.playTurnOfComputer() START
- Logic.AppPrivateAction.startComputerThinking
- Logic.AppAction.playTurnOfComputer() END
- Logic.AppAction.waitForPlayer() END
- Logic.AppAction.saveGame() START
- Logic.AppPrivateAction.finisedSaveGame
- Logic.AppAction.saveGame() END
- Logic.AppPrivateAction.endComputerThinking
▿ Logic.AppAction.placeDisk
▿ placeDisk: Logic.PlacedDiskCoordinate
- disk: Logic.Disk.diskLight
▿ coordinate: Logic.Coordinate
- x: 2
- y: 2
- Logic.AppAction.nextTurn() from: sideLight to: sideDark
- Logic.AppPrivateAction.nextTurn
- Logic.AppPrivateAction.validateTurn
@01234567
0--------
1--------
2--ox----
3---ox---
4---xo---
5--------
6--------
7--------
- Logic.AppAction.waitForPlayer() START
- Logic.AppAction.playTurnOfComputer() START
- Logic.AppPrivateAction.startComputerThinking
- Logic.AppAction.playTurnOfComputer() END
- Logic.AppAction.waitForPlayer() END
- Logic.AppAction.saveGame() START
- Logic.AppPrivateAction.finisedSaveGame
- Logic.AppAction.saveGame() END
- Logic.AppPrivateAction.endComputerThinking
▿ Logic.AppAction.placeDisk
▿ placeDisk: Logic.PlacedDiskCoordinate
- disk: Logic.Disk.diskDark
▿ coordinate: Logic.Coordinate
- x: 5
- y: 4
CI/TEST
GitHub ActionでXcodeのビルドとテストを実施できるようにしました。
# .github/workflows/build.yml
on:
push:
branches:
- 'master'
pull_request:
branches:
- '**'
env:
project_nmae: Reversi
scheme: Reversi
configuration: Debug
name: Xcode build
jobs:
validate:
name: Validate
runs-on: macOS-latest
strategy:
matrix:
destination:
- "platform=iOS Simulator,OS=13.4.1,name=iPhone 11 Pro"
steps:
- name: Checkout
uses: actions/checkout@master
- name: Switch to workspace directory
run: cd $GITHUB_WORKSPACE
- name: Install tooling
run: sudo xcode-select -s /Applications/Xcode_11.4.1.app
- name: Resolve swift package dependencies
run: xcodebuild -resolvePackageDependencies -scheme '${{ env.scheme }}' -clonedSourcePackagesDirPath ./.swiftpackages -derivedDataPath ./.build
- name: Run tests ${{ matrix.destination }}
run: xcodebuild -sdk iphonesimulator -scheme '${{ env.scheme }}' -configuration '${{ env.configuration }}' -destination '${{ matrix.destination }}' -clonedSourcePackagesDirPath ./.swiftpackages -derivedDataPath ./.build clean test | xcpretty
既知の不具合への対処
本チャレンジではリファクタリング対応にとどまらず、潜在する不具合を発見し的確に対処する必要があります。
手詰まりしたときの要対応事象 その1
手詰まりになった場合、アラートが表示された状態でアプリを終了すると、再度アプリを起動して前回終了時をロードしたときにクラッシュします。
手詰まりしたときの要対応事象 その2
以下のように、ひとまずクラッシュしないように改修すると、今度は再度アプリを起動して前回終了時をロードしたときに手詰まりのアラートが表示されず当該ターンではディスクを置けない状態に陥ります。
diff --git a/Reversi/ViewController.swift b/Reversi/ViewController.swift
index 321824a..411c06b 100644
--- a/Reversi/ViewController.swift
+++ b/Reversi/ViewController.swift
@@ -285,6 +285,7 @@ extension ViewController {
/// "Computer" が選択されている場合のプレイヤーの行動を決定します。
func playTurnOfComputer() {
guard let turn = self.turn else { preconditionFailure() }
+ guard !validMoves(for: turn).isEmpty else { return }
let (x, y) = validMoves(for: turn).randomElement()!
リセットアラートの表示時の要対応事象
リセットボタンを押下し、リセットの確認アラートを表示している状態でもコンピュータの操作は可能なのでゲームは進行していきます。このとき、手詰まりが発生すると、手詰まりのアラートを表示すべきところですが、すでにリセットの確認アラートを表示しているためリセットの確認アラートを表示できず操作ができない状態に陥ります。
リグレッションしないように気を付けるところ
- 1. コンピュータが思考中にリセットした場合、リセット後のゲームでコンピュータの思考中が継続され誤ってディスクを指さないこと
- 2. フリップアニメーション中にリセットした場合、リセット後のゲームでフリップアニメーションが継続されないこと
| 1. | 2. |
|---|---|
 |
 |
まとめ
反省点
- Reduxアーキテクチャに移行にあたって、リファクタリングがおおむね完了するまでテストコードを導入できなかった
- 移行が完了しないとReduxを活かしたテストコードを導入できなかったため
- テストコードでリファクタリングの過程にリグレッションしないことを担保したかった
- リファクタリング前と後でコードステップ数を計測すると67％増加していた
- Reduxのメリットの堅牢を求めるあまり、複雑なコードになったり過剰なリファクタリングとなった部分が否めない
コードステップ数
| - | リファクタリング前 | リファクタリング後 |
|---|---|---|
| ステップ数 | 775 | 1,295 |
未完成部分
今後の課題です。
- テストコードの記述がほとんどありません、もっとテストを書くべき！
- システム的な異常系のハンドリングは
preconditionFailureとassertionFailureの活用にとどまっており、ユーザーに異常状態を通知できていない
感想
リバーシという題材のチョイスがよく程よいボリューム感とリバーシロジックの難易度があり、ベースコードのファトコードの再現具合も絶妙で多様なアーキテクチャでリファクタリングのアプローチが可能なうえ、噛めば噛むほど味の出るすばらしいチャレンジだと感じました。