Slug andy controllers cover

Let's Play: 巨大ビューコントローラをリファクタリングしよう!

さて、ここになんでも知っている巨大なビューコントローラがあります。それは黒幕となり、ディスクI/Oとナビゲーションバーのスタイリングを同時に扱うほどに肥大化してしまいました。Andy Matuschakはライブコーディングという武器を手に、野獣のようなビューコントローラのサイズを減らし、その大きくなりすぎた役目をリファクタリングして、世界の破滅を防ぎます。さあ、はじめよう!


イントロダクション

Andy Matuschakです。今日は、とてもひどいビューコントローラをリファクタリングするデモを行います。私はKhan Academyでモバイル開発のリードをしていて、誰でもどこからでも無料で受けられる世界規模の教育を提供しようとしています。その前は、何年もの間、AppleでUIKitの開発に取り組んでいました。なので、UIViewControllerをネタにしても許されるんじゃないかと思ってます。

オリジナルの巨大ビューコントローラとリファクタリングのステップはGitHubのリポジトリにあります。これを使って、進めていきましょう

ひどいビューコントローラ

さて、これは私の作ったモックアプリ、MegaControllerといいます。これはToDoリストです。タスクを追加したり、完了したら削除することができます。タスクを完了するまでにどのくらい猶予があるかによって、それぞれ見出しをつけてタスクを分類します。トップのナビゲーションバーの色はどれだけ追い込まれているかの度合いによって変わり、もし忙しかったら赤(“Danger”:危険)、そうでなければ黄色(“Warning”:警告)、あるいは白(“Placid”:平穏)になります。きわめてシンプルなアプリです。

このビューコントローラは4つのことをしていて、ひとつのクラスとしては役割が多すぎます。

class ViewController: UITableViewController, NSFetchedResultsControllerDelegate, UIViewControllerTransitionDelegate, UITViewControllerAnimatedTransitioning

このひどいビューコントローラのクラスを見てください。クラスがやっていることを書き出してみました。

  • FetchedResultsController
  • Core Dataスタック
  • NSPredicateとNSFetchedRequestの設定
  • I/O処理
  • 外観と表示の管理
  • 表示のための大きな変換
  • TableViewDelegate
  • TableViewDataSource
  • セクションのタイトルを管理
  • セルの表示と密結合
  • フェッチ結果の動的な変更のハンドリング
  • 他のビューコントローラのためのカスタムアニメーション
  • 凶悪なUnwind処理の結合っぷり

いくつかについて詳細を見てみましょう。

FetchedResultsController

このビューコントローラはFetchedResultsControllerを持っていて、これの設定をし、またそのデリゲートとなっています。ファイルシステムからデータを読み込んで、Core Data関連の全ての処理を始めています。これはCore Dataのテンプレートですね。このビューコントローラの中で使うCore Dataスタックを起ちあげます。

ViewDidLoad

viewDidLoadでは、きわめて複雑なNSPredicateとNSFetchedRequestを組み立てています。FetchedResultsControllerを設定して、フェッチを実行させます。なので、I/O処理をしています。それから、賢いキャッシュ処理的なものですね、これはまだはっきりとはわかりませんが。ナビゲーションバーとステータスバーの更新もやっているので、外観と表示の部分にも関わっています。またフェッチした結果を、テーブルビューのセクションに日付に応じて動的に流しこむようなコードも含んでいます。“soon”(もうすぐ)か“upcoming”(これから)というセクションを、任意の数字で定義しています。ここで、表示を変えるための大きな変換をしています。また、このビューコントローラはテーブルビューのデリゲートであり、テーブルビューのデータソースでもあります。さらに、セクションタイトルの文字列を管理しています。またセクションのインデックスが0だったらタイトルは“今(now)”のように結びつけることも行っています。

Table View

//cellForRowAtIndexPath
switch numberOfCalendarDaysUntilTaskDueDate {
  case 0:
    description = "Today"
  case 1:
    description = "Tomorrow"
  default:
    description = "In \(numberOfCalendarDaysUntilTaskDueDate) days"
}

cell.detailTextLabel.text = description.lowerCaseString
return cell

まず、この部分が、セルの右側に表示される“Today”(今日)のような文字列を決めています。でも、バグがあります。昨日締め切りのタスクには“Yesterday”(昨日)と表示されるべきですが、そのためのコードは書かれていないので、“−1日”と表示されています。また、このコードはセルの表示処理と密結合しています。セルの中身に手を出して、detailTextLabelのtextプロパティを設定しています。またセルのtextLabelのtextプロパティも設定しています。セルが情報を表示する方法を変えたい場合は、このコードも変えなくてはいけません。ですので、ここもまたリファクタリングし役割を外部化する必要があります。

動的な変更のハンドリング

また別の持つべきでない役割として、フェッチ結果の動的な変更のハンドリングがあります。

func controller(controller: NSFetchedResultsController, didChangeObject anObject: NSManagedObject, atIndexPath indexPath: NSIndexPath?, forChangeType type: NSFetchedResultsChangeType, newIndexPath: NSIndexPath? ) {
  switch type {
    case .Insert:
      //insert task to table view
    case .Delete:
      //remove from table view
  }
}

FetchedResultsControllerは挿入されたオブジェクトを返すか、あるいはオブジェクトを削除したことを教えてくれますが、今度はあなたがテーブルビューにその情報を伝えに行かなければいけません。ここでは、これらの動的セクションのせいで、事態がさらに複雑になっています。FetchedResultsControllerはこれらのセクションの計算ができません。本来ならManaged Objectのプロパティに保存されるはずの情報だからです。この部分のコードはFetchedResultsControllerが知っているインデックスパスを元に、ユーザーによって削除されたり挿入されたりしたセルが行くべき“soon”や“upcoming”セクションの中で表示されるインデックスパスを計算しています。

ナビゲーションバーとタスクの追加

ナビゲーションバーとpreferredStatusBarStyleについての処理は、外観と表示の両部分を担当しています。また、“plus(+)”ボタンが押された時にカスタムアニメーションを行うので、Segueに関する処理も入っています。そのため、他のビューコントローラを表示するカスタムアニメーションを提供しています。このビューコントローラは、他のビューコントローラがどのように表示されるべきかについて密結合された知識を持っています。

記事の更新情報を受け取る

凶悪なUnwind処理の結合っぷり

最後の1つはunwindFromAddControllerです。これは特に凶悪です。これはIDから作られたアクションです。Segueから元のコントローラを取り出し、AddViewControllerを通して強制キャストさせます。これは、この関係のなかで密結合している部分です。そのうえ、そのビューコントローラの持つUI部品のプロパティから直接データを取り出しています。きっとみなさんのアプリにもこんなコードが入っていますよね。さらにCore Dataの永続化レイヤも直接変更しています。したがって、このひとつのメソッドには、モデル、ビュー、コントローラが個々に行うべき処理がすべて、1箇所に詰まっているのです。

Let’s Play: やつらを直せ

では、これらに取り組んで、テストを書いていきましょう。ある人たちは、全てをモック化・スタブ化して、ビューコントローラ内で行われるであろう全ての操作を自動化するのが良いと思うかもしれません。それも良いですが、私としては、そこで起こる実際のロジックをテストしたいと思いますので、そういうテストに集中していきます。XMLっぽく見える部分については、テストしません。ビューコントローラはXMLではありませんが、ときどき、このビューコントローラではないかもしれないけど、ビューコントローラのメソッドの中には計算をしないものがあります。

ナビゲーションバーのテーマ設定

例えばこんな方法で、Swiftらしい面白いアプローチで、ナビゲーションバーとステータスバーのスタイルをリファクタリングして、テーマ設定をビューコントローラの外に出すことができます。 新しいenumのNavigationThemePlacidWarningDangerのどれかの値を取ります。

enum NavigationTheme {
  case Placid
  case Warning
  case Danger

  var barTintColor: UIColor? {
    switch self {
      case .Placid: return nil
      case .Warning return UIColor(red: 235/255, green: 156/255, blue: 77/255, alpha: 1.0)
      case .Danger return UIColor(red: 248/255, green: 73/255, blue: 68/255, alpha: 1.0)
    }
  }
}

Objective-Cでは、enumはただの数字ですが、Swiftではもっとしゃれたことができます。このNavigationThemeに、たとえば、barTintColorを持たすことができます。これで、純粋なメソッドとなり、独立したものになりました。 これはnavigationTheme内に持たせることにできるロジックのおもしろいところであり、これによって今の状況がPlacidなのかDangerなのかわかるようになります。このロジックはエクステンションとして書きます。なぜなら、この処理を別のファイルに書けることを強調したいからです。

extension NavigationTheme {
  init(forNumberOfUpcomingTasks numberOfUpcomingTasks: Int) {
    switch numberOfUpcomingTasks {
      case 0...3:
        self = .Placid
      case 4...9:
        self = .Warning
      default:
        self = .Danger
    }
  }
}

もし我々がJavaで書いていたら、これはファクトリーになったかも知れませんが、いま書いているのはJavaではありません。SelfはPlacidであり、よりSwiftらしい慣用的なスタイルであるカスタムコンストラクタとして書いています。この処理をあえてextensionとして書くことで、この型自体の実装は、コンストラクタについて何も知らなくてよいという事実を強調しています。またこのようにコンストラクタを書くことで、もしNavigationTheme型のプライベートな詳細があったとしても、コンストラクタではその情報を絶対に参照しないように最善を尽くしています。これは、NavigationThemeFactoryにしたかったら外部の型にもできますが、Swiftではそうしません。

まだ他のスタイリング処理は実装していませんが、これはわざとです。というのも私はテストを書きたいからです。すでに、我々は意味のあるテストを書ける段階にいます。たとえば、testNoTaskIsPlacid。私はXcodeのユニットテストフレームワークを使っていて、またSwift 2で新しく導入された素敵なものを使っています。Swift 2では、インターナルな処理でも@testableを使うことでテストできるのです。NavigationThemeはインターナルであることに留意してください。通常、他のモジュールからは参照することができませんが、@testableのおかげで参照できます。ここでは、もしタスクが0個の場合NavigationThemeはPlacidになることをアサーションできます。

//NavigationThemeTests.swift

@testable import MegaController
import XCTest

class   NavigationThemeTests: XCTestCase {
  func testNoTaskIsPlacid() {
    XCTAssertEqual(NavigationTheme(forNumberOfUpcomingTasks: 0), .Placid)
  }
}

ご存知のとおり、さらに、いくつかのタスクがあればWarningとなることや、タスクがとても多い場合にDangerになることもテストできます。我々のテストはパスしたので、ドメインの知識はこれでわかったということで、安心してnavigationThemeの処理に戻ることができます。

デザイナーのくれた仕様では「こういう状況のときはPlacid状態となる」ということなので、ここではそれを実装しています。いまのテストはこのドメインロジックについての実装をテストしていたというわけです。これからはNavigationThemeの残り部分を実装します。titleTextAttributesはObjective-Cの国から来たとても美しい型です。もし我々がPlacid状態のとき、特別なtitleTextAttributesはいりませんが、WarningやDangerのときはテキストを白くします。同じようにして、tintColorも実装します。

enum NavigationTheme {
  case Placid
  case Warning
  case Danger

  var barTintColor: UIColor? {
    switch self {
      case .Placid: return nil
      case .Warning return UIColor(red: 235/255, green: 156/255, blue: 77/255, alpha: 1.0)
      case .Danger return UIColor(red: 248/255, green: 73/255, blue: 68/255, alpha: 1.0)
    }
  }

  var titleTextAttributes: [String: NSObject]? {
    switch self {
      case .Placid: return nil
      case .Warning, .Danger:
        return [NSForegroundColorAttributeName: UIColor.whiteColor()]
    }
  }

  var tintColor: UIColor? {
    switch self {
      case .Placid: return nil
      case .Warning, .Danger:
        return UIColor.whiteColor()
    }
  }

  var statusBarStyle: UIStatusBarStyle {
    switch self {
      case .Placid: return .Default
      case .Warning, .Danger: return .LightContent
    }
  }
}

バーがPlacidのときはtintColorは無しで、右上のプラスボタンにはシステムデフォルトの青を使います。でも、WarningやDanger状態のときは、白を使います。またpreferredStatusBarStyleも、ちょっとおもしろいのでお見せしましょう。Placidのときは、システムデフォルトのスタイルを使い、Danger領域のときはlight contentを使います。

さて、このようにナビゲーションテーマのコードはXMLっぽくなりました。よって、ビューコントローラの中から該当するコードを取り除きたいと思います。このようにコードを交換し、移動させるわけです。特に重要なのは、他のものから隔離されたところに移動する、ということです。このNavigationThemeのファイルは、プログラムの他の部分と関係なくなりました。そして、私が実行しろと言わないかぎり実行されません。さあ、これで、ステータスバーの時間部分が変わっただけで実行されてしまうビューコントローラのコードとは全く別物になりました。時間部分が変わったからといって実行されるのはナビゲーションのテーマとしては正しくありません。だからこそ私は、こういうスタイルのコードが好きなのです——これで、このファイルを理解して把握できるようになります。

私はこの気味の悪いものをビューコントローラから追い出さなければなりません。このビューコントローラは、ナビゲーションコントローラがどうなっているかを知る権利はないはずです。さらに、厚かましくも、このビューコントローラは親のナビゲーションコントローラの中にまで首を突っ込んでいます。もしくは周りの他のコンテナビューコントローラかも知れません。ビューコントローラが何だろうが知ったこっちゃありません。しかも、こいつはナビゲーションバーの色を変えようとしています。もし他のビューコントローラもナビゲーションバーの色を変えようとしていたら?アプリの中の別々の2箇所で同じことをしようとして、完全にめちゃくちゃになってします。最初の段階で、この部分はひどい状態でした。

ステータスバーのスタイル

またpreferredStatusBarStyleがあります。これは興味深いです。というのもナビゲーションコントローラは通常、その子にpreferredStatusBarStyleが何かを訊かないからです。なので、ナビゲーションコントローラをオーバーライドし、その処理をさせる必要がありました。

//NavigationController.swift

class NavigationViewController: UINavigationViewController {
  override func childViewControllerForStatusBarStyle() ->
  UIViewController? {
    return topViewController
  }
}

あまりお勧めできないような挙動を取り除くために、私が数年前にWWDCで喋った、ある原則に基づいて作業したいと思います。私が自分自身にしようとした質問は次のとおりです。 「真実はどこだ?ナビゲーションバーの色について最高責任を持つのは誰だ?誰がそれを知るべき?」 私は、これらのもの両方の親となるものを提案します。両方に共通の親がいれば、これらがお互いを知ることを避けられます。ナビゲーションコントローラが、その中のトップビューコントローラの正体について知るべきである理由は、特にありません。ナビゲーションコントローラのクラスは私のビューコントローラのクラスと密結合するべきではないですし、もちろんその逆もしかりです。これが意味するのは、外部に仲裁者が必要だということです。通常はAppDelegateになるでしょう。一見するとひどいアイデアに見えますが、これがひどいアイデアにならないように私はベストを尽くします。これについては後で、一緒に判断しましょう。

この、真実を知っている外部の存在を実現するためには、その存在にイベントを疎結合な方法で教えてあげられる必要があります。私はこれを非常に愚直な方法で行います。themeDidChangeHandlerです。

var themeDidChangeHandler: ((NavigationTheme) -> Void)?
var theme: NavigationTheme {
    return NavigationTheme(forNumberOfUpcomingTasks: fetchedResultsController?.fertchedObjects?.count ?? 0)
}

ナビゲーションテーマが変わるたびに、これを呼びます。それに加えて、現在のテーマ、ナビゲーションテーマへのアクセサを用意します。これを計算するのはとても簡単です。なぜならupcomingのタスクの数に対するナビゲーションテーマを計算するだけだからです。upcomingのタスクの数はFetchedResultsControllerのfetchedObjectCountsで取得できます。あるいは、タスクが存在しない場合は0にします。fetchedObjectsはオプショナルだからです。ナビゲーションバーを更新する際には、代わりに、themeDidChangeHandlerを自分のテーマを引数に呼び出します。

//ViewController.swift
...

func updateNavigationBar() {
  themeDidChangeHandler?(theme)
  ...
}
...

これで、この部分は、自分自身が入っているナビゲーションコントローラについて何も知らなくなったし、どうやってナビゲーションバーを変えるかについても知りません。私がストーリーボードを好きな理由のひとつは、単純なケースであれば、あるビューコントローラが他のビューコントローラを知っているという結合を取り除いてくれるからです。ひとつお見せしたいのは、このプラスボタンを押すと、AddViewControllerが表示されるにもかかわらず、そのためのメソッドはない、ということです。presentAddViewControllerのようなメソッドはありません。プラスボタンが押された時に表示されるビューコントローラの正体についてはストーリーボード内にエンコードされています。これはクールな疎結合の方法です。 しかしながら、これらのビューコントローラのインスタンスを参照したいのに、できません。これからやるのは、現時点でのハックにすぎません。いくつかの仮定をしてみましょう。たとえば、ナビゲーションコントローラはルートビューコントローラである、そして我々はウィンドウを持っている、というように。

//AppDelegate.swift
class AppDelegate: UIResponder, UIApplicationDelegate {
  var window: UIWindow?

  private var navigationController: NavigationController {
    return window!.rootViewController! as NavigationController
  }

  private var storyboard: UIStoryboard {
    return UIStoryboard(name: "Main", bundle: nil)
  }

  private lazy var viewController: ViewController = {
    return self.storyboard.instantiateViewControllerWithIndentifier("Primary") as! ViewController
  }()

}

ビューコントローラのインスタンス化をストーリーボードから切り離すために、最終的には、起動時にナビゲーションバーの色を変えるthemeDidChangeHandlerを作ります。

//AppDelegate.swift
...
func application(application: UIApplication, didFinishLaunchingWithOptions launchOptions: [NSObject : AnyObject]?) -> Bool {
  viewController.themeDidChangeHandler = { [weak self] in
    if let navigationController = self?.navigationController {
      //avoid this
      navigationController.navigationBar.barTintColor = theme.barTintColor
    }
  }
  navigationController.viewControllers = [viewController]
  return true
}
...

さて、テーマを変える時には、themeDidChangeHandlerはこのようなコードになります。ナビゲーションコントローラを取り出すとしたら、weakなので、このようなステップを踏まなければなりません。ここで私がやりたくないのは、navigationbar.barTintColor = theme.barTintColorのようなことです。これは心地良くありません。このようなことが4つ続いてしまい、もはやXMLのようなスタイル定義ではなくなって���まいます。これを避けるために、もうひとつファイルを作ります。ここでの戦略は、分解に尽きます。今からナビゲーションバーにテーマを適用するメソッドを作ります。

//UINavigationBar-Theme.swift

extension UINavigationBar {
  func applyTheme(theme: NavigationTheme) {
    barTintColor = theme.barTintColor
    tintColor = theme.tintColor
    titleTextAttributes = theme.titleTextAttributes
  }
}

//AppDelegate.swift
func application(application: UIApplication, didFinishLaunchingWithOptions launchOptions: [NSObject : AnyObject]?) -> Bool {
  viewController.themeDidChangeHandler = { [weak self] in
    if let navigationController = self?.navigationController {
      navigationController.navigationBar.applyTheme(theme)
    }
  }
  navigationController.viewControllers = [viewController]
  return true
}

このファイルは完全に全てから切り離されています(ただしUIKitを除いてですが——ここでわたしができる妥当なことはありません。みんながReact Nativeを使い出して、UIKitを完全に見捨てるまではね)。さて、これで、適切なバーの色とテーマが準備できたと思います。

ナビゲーションバーは黄色になりました。でもステータスバーの色が正しくありません。今からやるのは、このchildViewControllerForStatusBarStyleのハックを移動させ、ステータスバーのスタイルの選択はそのトップビューコントローラが担当するべきだ、ということをナビゲーションコントローラに知らせることです。またトップビューコントローラは、ステータスバーの色を変えたいときに親ビューコントローラのsetNeedsStatusBarApperanceUpdateを呼び出す責任があります。ですが、これでは前と同じになってしまうので、かわりにデータ駆動にしましょう。

//NavigationController.swift

class NavigationViewController: UINavigationViewController {
  var statusBarStyle: UIStatusBarStyle = .Default  {
    didSet {setNeedsStausBarAppreanceUpdate()}
  }

  override func preferredStatusBarStyle -> UIStatusBarStyle {
    return statusBarStyle
  }
}

//AppDelegate.swift
...
func application(application: UIApplication, didFinishLaunchingWithOptions launchOptions: [NSObject : AnyObject]?) -> Bool {
  viewController.themeDidChangeHandler = { [weak self] in
    if let navigationController = self?.navigationController {
      navigationController.navigationBar.applyTheme(theme)
      navigationController.statusBarStyle = theme.statusBarStyle
    }
  }
  navigationController.viewControllers = [viewController]
  return true
}

今やった方法の何が良いかというと、真実が明確に表現されていることです。誰もこのナビゲーションコントローラのステータスバーのスタイルや、バーの色をめぐって喧嘩しません。代わりに、我々はデータと管理について明確なフローを、ツリー状の形で手にしました。これは私の好きな形です。私は、プログラムがとても理解しやすいときとは、流れる管理フロー、権限のフロー、データのフローが全てまっすぐにツリー構造を登り降りしているときだと思います。さて、このビューコントローラについての本当にひどいリストのひとつをやっつけたので、ようやく「完了」としてマークすることができます。

データソースの分離

では次に、データソースの状況をなんとかしてみましょう。まず最初に、このCore Dataについての全ての処理を他のファイルに分離したいと思います。この処理をCore Dataスタックと呼ぶことにします。

//CoreDataStack.swift

import CoreData
import Foundation

class CoreDataStack {
  lazy var applicationDocumentsDirectory: NSURL = {
    let urls = NSFileManager.defaultManager().URLsforDirectory(.DocumentDirectory, inDomains: .UserDomainMask)
    return urls[urls.count-1]
  }()

  lazy var managedObjectModel: NSManagedObjectModel = {
    let modelURK = NSBundle.mainBundle().URLForResource("MegaController", withExtension: "momd")!
    return NSManagedObjectModel(contentsOfURL: modelURL)!
  }()

  lazy var persistentStoreCoordinator: NSPersistentStoreCoordinator = {
    let coordinator = NSPersistentStoreCoordinator(managedObjectModel: self.managedObjectModel)
    let url = self.applicationDocumentsDirectory.URLByAppendingPathComponent("SingleViewCoreData.sqlite")
    do {
      try coordinator.addPersistentStoreWithType(NSSQLiteStoreType, configuration: nil, URL: url, options: nil)
    }catch{
      fatalError("Couldn't load database: \(error)")
    }

    return coordinator
  }()

  lazy var managedObjectContext: NSManagedObjectContext = {
    let coordinator = self.persistentStoreCoordinator
    var managedObjectContext = NSManagedObjectContext(concurrencyType: .MainQueueConcurrencyType)
    managedObjectContext.persistentStoreCoordinator = coordinator
    return managedObjectContext
  }()
}

ここで、マネージドオブジェクトコンテキストだけをクライアントがアクセスできるものにします。もし、今これをテストしようとしても、インメモリの保存に関してなにもできないでしょう。ですが、少しずつやっていけば良いのです。私がこういうやり方でのリファクタリングが好きなのは、ひとつひとつ取り出していけば良いからです。ひとつ取り出すたびにより良くなり、よりわかりやすくなるからです。Core Dataスタックを取り出した後、ViewController内のものを直していきましょう。

//ViewController.swift

...
  private let coreDataStack = CoreDataStack()
...
  fetchedResultsController = NSFetchedResultsController(fetchRequest: fetchRequest, managedObjectContext: coreDataStack.managedObjectContext, sectionNameKeyPath: nil, cacheName: nil)
...
//replace all managedObjectContext with coreDataStack.managedObjectContext

このような処理は一行もこのファイルに残したくありませんが、全部はやりきれません。でも、ここで私がやっていることは、少しずつの改良です。前は、4つの変数が誰からでもアクセスできてめちゃくちゃにできる状態で置かれていましたが、今ではひとつしかありません。さらに厳しい管理を、初期化のタイミングでできます。この線で進めていく場合、私が次にする改良は、Core Dataスタックを使う処理をクラスにまとめることになるでしょう。つまり、Core Dataスタックのインスタンス化をこのビューコントローラから取り除けるということです。これは素晴らしいことです。というのも、プロジェクト内で他のビューコントローラを作り、それにもCore Dataスタックを与えるのは非常に自然なことだからです。その点では、こういったコードをビューコントローラ内に持っているのは非常に不適切です。

ですが、Core Data関連のドロドロしたものがこのファイルにはまだたくさんあります。ちょっと時間をとってCore Dataの文句を言いますが、それは、ここで問題となっている本質のほうが、私がコードを変えていくのを見るよりも役立つからです。そのドロドロは、Core Dataの奥深くにあります。この奥深くから続いているドロドロは、よく文句を言われるCore Dataの特定のデザイン決定、例えば「APIサービスが大きすぎる」「全てのメソッドが複雑すぎる、あるいは不完全」というようなものが原因ではありません。問題の根本は、共有されている変更可能な状態がCore Data内のどこにでもあることです。もしあなたがNSManagedObjectを持っているとすると、そのNSManagedObjectは、それを作り、管理しているNSManagedObjectContextへの参照を持っています。そのマネージドオブジェクトへの参照を持っているものなら誰でも、マネージドオブジェクトを変更可能です。また、マネージドオブジェクトが来たところから、マネージドオブジェクトコンテキストへの参照も可能です。もしあなたが注意深くなければ、何でも起こってしまう可能性があるのです。けれど、注意深くなるための良い方法はありません。私にとっては、これがCore Dataや他のORMのめちゃくちゃ腹立たしいところです、本当に。

次の、私にとって自然なステップは、このビューコントローラ内からCore Dataに関する知識を徐々に取り除いていくことです。私がどのようにやっていくかについて、ある程度の概略があるので、それをお話しします。このクラスの中にまだあるのは——リストをもう一度見てみましょう——FetchedResultsController、これはCore Dataと結合しています。Core Dataスタックは片付きました。NSPredicateとNSFetchedRequestですが、これもCore Dataに結合しています。I/O処理、保存関係、はまだクラス内にあります。またフェッチ結果の動的な変更のハンドリングもあります。まず、この役割のひとつひとつを取り除いていくところからはじめようと思います。最後の最後にやることはNSManagedObjectを取り除くことです。というのもNSManagedObjectは、それが触れるすべてのものとの間に依存関係を生み出すからです。そして、得られるものが表示に関して必要なデータだけになるまで、取り除くのを続けるでしょう。またタスクを作りたくなったときには、タスクを作るのに必要な物を提供するだけにしたいと思います。これが、Core Dataのオブジェクトを作る方法です。ここでは、NSManagedObjectContextを取得し、NSPersistentStoreCoordinator、NSManagedObjectModelを取得し、文字列“Task”へのエンティティを取得し、コンテキストに挿入しています。

let newTask = NSManagedObject(entity: coreDataStack.managedObjectContext.persistentStoreCoordinator!.managedObjectModel.entitiesByName["Task"]!, insertIntoManagedObjectContent: coreDataStack.managedObjectContext)
newTask.setValue(addViewController.textField.text, forKey "title")
newTask.setValue(addViewController.datePicker.text, forKey "dueDate")
try! coreDataStack.managedObjectContext.save()

ここは、どう見てもひどく書きすぎました。NSManagedObjectのサブクラスを作ることができたはずです。NSManagedObjectのサブクラスはこれらの処理全てと結合していますが、それを隠しているのです。とはいえうまく隠しているとはいえません。

どうあるべきだったかのヒントをお見せしましょう。私はDataManagerというクラスを作りました。これもまだマネージドオブジェクトの観点で実装されていますが、薄れつつあります。このデータマネージャを使った場合のビューコントローラの状態を簡単に見てみましょう。FetchedResultsControllerはいなくなりました。データマネージャに入っているからです。また、大部分はかなりXMLっぽくなってきました。このビューコントローラがまだテーブルビューのデリゲートとデータソースであるのは気に入らないですが、少なくとも実装としては何もやっていません。多くの知識をエンコードはしていません。また「NSFetchedResultsControllerの何かが変更されたよメソッド」の、より明確にした、親しみやすいバージョンのものもできました。ここでまったくNSManagedObjectを扱っていないことに気づくと思います。ですが、cellForRowAtIndexPathでセルをアップデートしているため、NSManagedObjectへの依存性はまだあります。

これを取り除くためには、残りのステップはひとつだけです。UpcomingTaskDataManagerは2つのプロパティを公開しています。ひとつはおもしろくて、テーマのために使ったタスクの数を数えるだけのもので、taskSectionsを公開します。このtaskSectionsは、マネージドオブジェクトのリストのリストです。このタスクのセクションの配列は、オブジェクトが挿入あるいは削除されるたびに動的に変わります。でも、これはストレージというわけではありません。マネージドオブジェクトの配列の配列の配列ではありません。実際にはresultCacheから来ています。これは構造体です。構造体が良い理由は、全てのものから完全に分離された状態まであと一歩の位置にあるからです。分離されてないただひとつの理由は、NSManagedObjectに依存しているからです。

どのようにNSManagedObjectに依存しているかを見てみると、具体的には、NSManagedObjectから引き出したいことは、締切日だけです。これは締切日のプロパティに対応するもののリストのリストにできるかもしれません。コンパイルは通るでしょう。この時点で、この全てのコードは、さっきナビゲーションテーマをリファクタリングしたのと同様の理由で、非常に納得がいくようになりました。すなわち、ここのコードは、直接の親が実行しろと命令するまでは実行されず、また変更不可能で、完全に分離されているからです。ここに“mutating”(変化する)という言葉があるのにimmutable(変更不可能)というのはおかしいと思うかもしれませんが、ここに、もう一つ興味深い原則があります。これは依然として変更不可能なのです。このインスタンスを持っている変数のスロットは変更可能ですが、そのインスタンス自体は変更不可能なのです。ですのでキャッシュから取り出してシリアライズしてファイルに保存することが安全にできますし、またテストに持っていくことも可能ですし、ネットワーク越しに送ることもできます。突然交換可能になったのです。

これらの全てはとても有効なことで、テストを書くのも非常に簡単にしてくれます。これから次のようなコードの変形を行います。まずNSManagedObjectを取り出し、つぎにこっちのプロパティのNSManagedObjectも取り出し、ようやく私のビューコントローラはNSManagedObjectの独裁から解放され、美しい世界がやってきます。やらなきゃいけないことは残りあと少しです。

テーブルビューとビューコントローラの分離

この部分をどのように取り除くか、すなわち、セルの設定と、ビューコントローラとテーブルビューセルの結合の部分について、これから簡単にお話します。ViewModelsというデザインパターンがあり、これがほとんど有効だと思います。このデザインパターンの仕組みは、ビューが自分自身をレンダリングするのに必要な情報だけを持つビューモデルというインスタンスへの参照を、ビューに与えるというものです。ビューモデルはビューのアクションも実装します。私があまりいいと思えないのは、ビューモデル自体が自力では動けず、いろんなものから管理されているという点です。最初の部分だけお話しすると、ViewDataというものを作ります。

class MyCell {
  struct ViewData {
    let title: String
    let timeSummary: String
    init(task: Task) {

    }
  }

  var data: ViewData {
    didSet {
      //set values of views
    }
  }
}

ここでインラインクラスを書きます。これをmyCellとして、myCellはViewDataを持ち、それはタイトルを持つかもしれません。これはレンダリングに必要なデータです。タイトルや、時刻の要約——“あと1日 (one day from now)”のようなものも持ちます。この2つの値は両方とも、自力では変更しないものです。生きたNSManagedObjectは渡しません。自分で何でもやってしまうようなものも渡しません。文字列の構造体を渡すだけです。ここで、この知識を、私のデータマネージャから出てきたものを元にしたViewData構造体のイニシャライザを作ることで、外に移動させることができます。いまはまだマネージドオブジェクトなので、マネージドオブジェクトからViewDataを初期化するか、もしくはそれが値型であるTaskになるなら、そこから初期化します。さて、イニシャライザは、この相対的なDateTimeのフォーマッタへの依存性をエン���ードするでしょう。これは私がこのスケッチの中に作ったもので、今日みなさんと作ったものではありませんが。これはみなさんの期待通りの処理をするものです、つまり日付から文字列を作り出します。繰り返しになりますが、これは完全に分離されています。

このロジックを抽出すると、次のようになります。セルはViewDataのセッターを持ち、didSetがすべきことは、ビューの値のアップデートだけです。ビューコントローラはセル内のビューについて知る必要はありません。ビューコントローラを変えることなく、セルの見た目を変更できるようになります。またモデルのエンティティを変えたり、タスクのフィールド変数を見たり、あるいは実装の本質を変えずに新しいタスクのフィールド変数の名前を変えることもできるでしょう。


ソースコードを読んで、GitHubのリポジトリのコミットヒストリーを通してみることで、さらにリファクタリングのステップについて学べます。


Q&A

Q: 構造体をディスクに保存することについて意見はありますか?NSKeyedArchiverNSCodingNSCopyingなどなどが必要ですが。

Andy: ちょうどブックマークのデータベースをシリアライズするものを書いたみたところです。何をやったかというと、シリアライザの構造体を作ったんです。それ自体は何もしないので構造体でいられます。データを返して、ブックマークを取ります。ただそれだけです。それは、ブックマークを持っているのが何かすら気にしません。その必要が無いからです。単純にプロパティリストのシリアライズを行い、それにブックマークをマッピングすることでエンコードします。それが期待されることだからです。

Q: ストーリーボードからコントローラをインスタンス化するとき、それはインスタンスだけを返します。ですので、プロパティが適切に初期化されているかを確認して共にルートビューコントローラをインスタンス化することができないと思うのですが、よい方法はありませんか?

Andy:ストーリーボードを使わないことでしょうか。アプリケーション全体をこのようにenumとして表したらどうでしょう?うまくいくかもしれません。そして、UINavigationViewControllerに応じていろいろと行うのではなく、ナビゲーションコントローラを、そのenumで表したアプリケーションの状態に従わせるのです。そうすれば、その状態に従ってビューコントローラをインスタンス化するだけです。ですが、UIKitと喧嘩することになるので気をつけて進めてください。

Q: React Nativeに言及していましたが、あなたがメインのUIフレームワークとして採用しない理由はなぜでしょう?

Andy: JavaScriptはひどいけれども、同時に未来でもあります、残念ながら。知る限りでは、我々はSwiftを採用した最初の大きなプロジェクトでした。2、3万行のSwiftコードを書き、ツールが動かなくなりました。我々は、テック系の企業ですらありません。Khan Academyは小さい非営利団体で、エンジニアは2〜3人しかいません。そして我々の問題は、テクニカルな問題ではありません。我々がツールを壊したのは、難しいことをやっているからではなく、アーリーアダプターだからです。この経験をした後、私はやや保守的になりました。次回は、誰かがReact Nativeの人柱になってくれるのを待ちたいと思います。

もうひとつ、Android版がまだ公開されていないという点があります。その結果として、もし採用したくても、利点がないのです。もし採用するときは(採用しそうではあります)、プレーンなJavaScriptは使わないでしょう。Flowを使って、型追跡やあるいはコルーチンなどなどを使い、何とか我慢して使えるようにすると思います。

Q: ビューコントローラをたくさんリファクタリングしましたが、どのポイントでやめよう、あるいは、他の人が読むとしたら抽象化しすぎたな、と感じますか?

Andy: これは大きな問題です。スパゲッティコードの逆をなんというか知っていますか?ラビオリコードといいます。面白い対比として、結合のバランスを表す凝集度という用語があります。結合は、多くの異質なものを同じものの中に入れておきたくない、ということです。凝集度への懸念とは、本質的に同じコンセプトのたくさんのものを多くの場所に散らばせておきたくない、ということです。

ここに悪いドキュメント的コメントの例があります。私は、最近これらにどうやって対処しているかをお話ししようとしていました。というのもこれで手こずっているからです。我々はこのドキュメント的コメントの中にたくさんの「e.g. (例)」を書いています。「これはあのUI部品のためのこれによって使われている」など。それで、結合——ではなくて、ドキュメント結合は、ひとつのコンポーネントを探すときに見る地図のようなものです。どこに行くべきかを指すポインターがあり、隣のコンポーネントなどを見ます。そうやって、地理がわかってきます。とはいえ、どの程度リファクタリングするかは重大な問題で、バランスを見つけるのは至難の業です。

翻訳:岩谷 明 Akira Iwaya
校正・校閲:Yuko Honda Morita

About the content

This content has been published here with the express permission of the author.

Andy Matuschak

Andy is the lead mobile developer at Khan Academy, where he’s creating an educational content platform that’s interactive, personalized, and discovery-based. Andy has been making things with Cocoa for 12 years—most recently, iOS 4.1–8 on the UIKit team at Apple. More broadly, he’s interested in software architecture, computer graphics, epistemology, and interactive design. Andy and his boundless idealism live in San Francisco, primarily powered by pastries.