プログラミング初心者のコードをレビューした時の内容を教えます2
前回に続いて初学者のコードレビューをたくさんやった経験から、同じようなことを何人にも指摘しました。今回も別の内容でまとめてみました。
今回はがっつりSwiftに関することです。
初学者の方に多いのがズバリ「Enumが正しく使いこなせてない」ことです。
今回も初学者が書きがちなコードを載せてそれをリファクタリングしていきます。
作りたい機能
単純に「男性」と「女性」のカード(セルっぽくなってしまったけど)を並べてタップしたら「これは男性です」「これは女性です」とアラートを出すだけです。(上の画像をタップするとアニメーションが見れます)
これをHumanのEnumを用意して作るとすると初学者の方々はこのように実装しました。
初学者のソースコード
まずはHumanです。
/****************************************/
/***************** 1 ********************/
/****************************************/
enum Human {
case man, woman
}次にカードのViewです。
//カード用のView
//必要なところだけ抜粋
final class CardView: UIView {
//外部から受け取るパラメータ
private(set) var human: Human = .woman
//カードに表示する要素
private let nameLabel: UILabel = .init()
private let iconView: UIImageView = .init()
override init(frame: CGRect) {
super.init(frame: frame)
self.setupView()
configure(human: self.human)
}
//外部からHumanパラメータを受け取る
func configure(human: Human) {
self.human = human
/****************************************/
/***************** 2 ********************/
/****************************************/
// humanでswitchして表示を変える
switch human {
case .man:
self.nameLabel.text = "男性"
self.iconView.image = UIImage.init(named: "man")
case .woman:
self.nameLabel.text = "女性"
self.iconView.image = UIImage.init(named: "woman")
}
}
private func setupView() {
//ラベルやアイコンのレイアウトを構築する
//...省略...
}
}最後にControllerです
import UIKit
final class ViewController: UIViewController {
//stackviewで並べます
@IBOutlet private weak var stackView: UIStackView!
//表示させるHumanです
let humans: [Human] = [.man, .woman]
override func viewDidLoad() {
super.viewDidLoad()
self.configureView()
}
private func configureView() {
//humansの数だけカードを並べます
for human in humans {
let cardView = CardView()
cardView.configure(human: human)
stackView.addArrangedSubview(cardView)
//カードごとにタッチのアクションを用意します
let tapGesture = UITapGestureRecognizer.init(target: self, action: #selector(tapCard(sender:)))
cardView.addGestureRecognizer(tapGesture)
}
}
//アラート用のメソッドです
private func showAlert(message: String) {
let alert = UIAlertController.init(title: nil, message: message, preferredStyle: .alert)
alert.addAction(UIAlertAction.init(title: "キャンセル", style: .destructive))
self.present(alert, animated: true)
}
@objc func tapCard(sender: UITapGestureRecognizer) {
guard let cardView = sender.view as? CardView else { return }
/****************************************/
/***************** 3 ********************/
/****************************************/
// カードがもつhumanでswitchしてアラートを出し分けます
switch cardView.human {
case .man:
showAlert(message: "これは男性です")
break
case .woman:
showAlert(message: "これは女性です")
break
}
}
}リファクタリング箇所
長々と書きましたが今回の要点はコメントで
「/****** 1 *****/」 「/****** 2 *****/」 「/****** 3 *****/」
を付けている３箇所です。
そこだけさらに抜粋します。
/****************************************/
/***************** 1 ********************/
/****************************************/
enum Human {
case man, woman
}final class CardView: UIView {
func configure(human: Human) {
self.human = human
/****************************************/
/***************** 2 ********************/
/****************************************/
// humanでswitchして表示を変える
switch human {
case .man:
self.nameLabel.text = "男性"
self.iconView.image = UIImage.init(named: "man")
case .woman:
self.nameLabel.text = "女性"
self.iconView.image = UIImage.init(named: "woman")
}
}
}final class ViewController: UIViewController {
@objc func tapCard(sender: UITapGestureRecognizer) {
guard let cardView = sender.view as? CardView else { return }
/****************************************/
/***************** 3 ********************/
/****************************************/
// カードがもつhumanでswitchしてアラートを出し分けます
switch cardView.human {
case .man:
showAlert(message: "これは男性です")
break
case .woman:
showAlert(message: "これは女性です")
break
}
}
}ご覧の通りView, Controllerともども、manかwomanでswitchして処理を切り替えてます。
これでちゃんと意図通りに動くのですが、これだと「Swiftが分かってないな」と思われ、iOSエンジニアとして採用するのは厳しかったです。
ちゃんとSwiftのEnumが分かっているとViewにもControllerにも一切Switchを書くことなく実装できます
リファクタリング後のコード
順番は逆ですが「/****** 2 *****/」 「/****** 3 *****/」のViewとControllerから記載します。
final class CardView: UIView {
func configure(human: Human) {
self.human = human
/****************************************/
/***************** 2 ********************/
/****************************************/
self.nameLabel.text = human.text
self.iconView.image = human.icon
}
}
}final class ViewController: UIViewController {
@objc func tapCard(sender: UITapGestureRecognizer) {
guard let cardView = sender.view as? CardView else { return }
/****************************************/
/***************** 3 ********************/
/****************************************/
showAlert(message: cardView.human.alertMessage)
}
}このようにどこにもswitchはありません。
しかし、どこかで表示する文言や画像を指定する必要があるのですが、それは全てenum側に記載されています。
/****************************************/
/***************** 1 ********************/
/****************************************/
enum Human {
case man, woman
// Humanの値ごとに表示するtextを出し分けるプロパティ
var text: String {
switch self {
case .man:
return "男性"
case .woman:
return "女性"
}
}
// Humanの値ごとに表示する画像を出し分けるプロパティ
var icon: UIImage? {
switch self {
case .man:
return UIImage.init(named: "man")
case .woman:
return UIImage.init(named: "woman")
}
}
// Humanの値ごとに表示するアラート文言を出し分けるプロパティ
var alertMessage: String {
switch self {
case .man:
return "これは男性です"
case .woman:
return "これは女性です"
}
}
}ご覧の通り、３つのcomputed propertyがenumに追加されています。
ViewやControllerではこのcomputed propertyが呼び出されていました。
リファクタリングの前のHumanだと、もし別のViewや別のControllerにHumanを利用するとしたらその度にアラート文言の出し分けるswitchを書く必要がありましたが、リファクタリング後だと全てがEnumに集約されているためViewやControllerにswitchをまた書く必要がありません。
ViewやControllerにSwitchの分岐がなくなるため、全体の処理が見やすくバグも起きづらくなります。
このようにSwiftのEnumは機能が豊富なのでそれを使いこなして実装すると「Swiftが分かっている」感が出せて採用もされやすくなると思います。
まとめ
・ViewやControllerにSwitchがあったらEnumのプロパティやファンクションで集約できないか考えよう
最後に今回のコード全文のリンクを置いておきます。
投げ銭程度で支援してくれると嬉しいです！！
ご支援ありがとうございました！！！！！
この記事が気に入ったらサポートをしてみませんか？