review_1016

[Kakeshin]

指摘事項

• ディレクトリ構成がおかしい

• View まとめて見にくい

• MARK コメント

• SelectImageCollectionViewCell, CatalogTableViewCell これ何してんの

• 不要な改行多い

• インデントおかしい

• EditDetailWordVC

  func dismissScreen() {
      if let navigationController = self.navigationController {
          navigationController.popToRootViewController(animated: true)
          self.delegate?.didDismissViewController(self)
          self.dismiss(animated: true)
      }
  }

  • なんでナビゲーションで遷移してるのに dismiss？

• ImageCreateVC

  extension CreateImageViewController: EditDetailWordViewControllerDelegate {
  func didDismissViewController(_ viewController: UIViewController) {
      if viewController is EditDetailWordViewController {
          self.wordTextField.text = ""
          ImageEditScreen.shared.deleteImageAll()
          self.editScreenView.subviews.forEach { subView in
              subView.removeFromSuperview()
          }
      }
  }
  }

  • View 同士を繋いでるけど MVP 破綻してね？
  • なんか循環参照しそう

• DetailWordVC

  if self.calledFlg == .LearningEnglish {
          self.dismiss(animated: true)
      } else {
          self.navigationController?.popViewController(animated: true)
      }

  • View でやることじゃない
  • フラグの命名ダサい

  func attachData() {
      self.englishWord.text = wordData.englishWordName
      self.japaneseWord.text = wordData.japanWordName
      self.englishSentence.text = wordData.englishSentence
      self.japaneseSentence.text = wordData.japanSentence
      self.wordImage.image = Converter().decodeBase64ToImage(wordData.imageURL)
  
      self.decideButton.setTitle("閉じる", for: .normal)
      // 呼ばれた画面次第で、表示する情報を変更する
      if self.calledFlg == .Catalog {
          self.editButton.setTitle("編集", for: .normal)
      } else {
          self.editButton.isHidden = true
      }
  }

  • プライベートなら private つけろよ

• ViewControllerFactory

  • DI じゃない
  • Factory って名前ダサい
  • Router とかでいいじゃん
  • 遷移までここで書いちゃえばいいじゃん

  static func editdetailWordViewController(_ wordData: WordData, _ image: UIImage?, _ viewController: UIViewController?) -> UIViewController {
      let vc = EditDetailWordViewController(wordData, image)
      vc.presenter = EditDetailWordPresenter(dataBaseRepository: DataBaseRepository())
      vc.delegate = viewController as? EditDetailWordViewControllerDelegate
      return vc
  }

  • 引数全部非表示になってるけど呼び出し元で分かり辛くない？

• SampleView

  • @objc メソッド全部 private でええやん

• DataBaseRepository

  do {
          return try dataBaseService.getAllWordData()
      } catch {
          throw myError.case1
      }

  • Repository でしてるのにする必要ある？

• SelectImagePresenter

  let urlString = "https://pixabay.com/api/?key=\(AccessTokens.share.pixabayAPIKey)&q=\(keyword)"

  • 環境変数にすれば？

[Kakeshin]

ぱっと見で気になるところだけ

異論は認めます

[takuto277]

修正内容

フォーマット

• 指摘項目/質問内容
  • 対応内容/回答

項目

• ディレクトリ構成がおかしい

  • 勉強し直します

• View まとめて見にくい

  • IBOutlet → Lifecycle → IBAction → private method → methodの準倍で統一します

• MARK コメント

  • 追々追加します

• SelectImageCollectionViewCell, CatalogTableViewCell これ何してんの

  • SelectImageCollectionViewCellは画像を検索するときのCollectionViewの画像表示
  • CatalogTableViewCellはタブバー中央のtableViewの表示Cell

• 不要な改行多い

  • 二行以上の改行は削除

• インデントおかしい

  • 全ファイルでCtrl + I

• EditDetailWordVC

  • dismissいらなかったです。修正済み

• ImageCreateVC

  • ①createImageVC→②EditDetailWordVCの順番で遷移して、②の処理が終わったら、popRootで①に戻ります。その①に戻った時に表示されているsubViewやtextをリセットするためのデリゲートプロトコルを作成して削除させてます。①はタブバーにあり、最初に生成されたインスタンスを使い回しているため、削除する処理が必要です。これ以外に方法が思いつかなったのが現状です。
  • DetailWordVC
  • presenterで分岐処理させるように修正
  • 修正に伴い、attachData()はprivateじゃなくなりました
  • SampleView
  • objc メソッドに全てprivateを追加
  • DataBaseRepository
  • 不要なエラーハンドリングは削除しました
  • SelectImagePresenter
  • 環境変数.envファイルを追加しました

[takuto277]

遷移までここで書いちゃえばいいじゃん

static func editdetailWordViewController(_ wordData: WordData, _ image: UIImage?, _ viewController: UIViewController?) -> UIViewController {
      let vc = EditDetailWordViewController(wordData, image)
      vc.presenter = EditDetailWordPresenter(dataBaseRepository: DataBaseRepository())
      vc.delegate = viewController as? EditDetailWordViewControllerDelegate
      return vc
  }

ここあんまり理解できてないです。 Viewでやってる繊維をstaticメソッドに設定することでしょうか