연락처 관리 앱 프로젝트 [STEP 2] Matthew, Asher #69
Conversation
|
안녕하세요 @tawans , @kimbs5899 ! 더불어서 target 브랜치�도 확인 부탁드립니다(현재는 diff를 확인하기가 어렵습니다) |
There was a problem hiding this comment.
안녕하세요 @kimbs5899 !
STEP2 진행하느라 고생 많으셨습니다!! 🥳🙌
작성해주신 코드들에 대해 라인별로 코멘트를 남겼는데요! 확인 부탁드리겠습니다.
여기서는 작성해주신 PR description에 대해 답변 드릴게요!
미리보기 기능을 위해 UIKit에서 preview를 쓰는지
회사마다 다를 수 있겠지만 저희는 잘 안씁니다 ㅎㅎ. 특별한 이유는 없고 빌드 시간이 오래 걸리는 편은 아니다보니 직접 실행해서 화면을 보는 것 같아요.
빌드 시간이 길어진다거나 depth가 깊어지는 화면이라면 고려해볼만한 방식인 것 같습니다!
extension 코드 분리에 대해
네 당연히 작성해주신 것처럼 분리해서 선언 및 사용합니다!
다만 개별 extension들 위에 MARK 주석을 달아서 아래 코드가 무엇에 대한 확장인지 간략하게 마킹합니다.
추가적으로, initializer와 loadView, viewDidLoad같은 부분은 같이 두기도 하는데(lifecycle) 이러한 부분은 선택(컨벤션)의 차이라 크게 의미를 두시진 않아도 될 것 같습니다.
작성해주신 배운점 관련하여
UUID에 대한 고민과 내부 저장에 대한 방향성 고민 (json, coreData)
이라고 써주셨는데 어떤 고민이었는지 조금 자세히 설명해주실 수 있으실까요? 궁금합니다!
UML 관련하여
UML 정말 잘 만들어주셨는데요! 궁금한 점이 몇가지가 있습니다.
- 클래스는 C라고 해주신 것 같은데 UIViewControllerRepresentable과 같은 것들은 Protocol이지 않을까요?
- 프로토콜들에 대한 채택은 inherit(상속)보다는 conform(준수)이라고 표기하는게 낫지 않을까요?
- 작성 툴은 어떤 것을 쓰신걸까요? (draw.io같은데 맞을까요?)
ios-contact-manager/Controllers/ContactListViewController.swift
Outdated
Show resolved
Hide resolved
ios-contact-manager/Controllers/ContactListViewController.swift
Outdated
Show resolved
Hide resolved
ios-contact-manager/Controllers/ContactListViewController.swift
Outdated
Show resolved
Hide resolved
|
@ictechgy UUID에 대한 고민과 내부 저장에 대한 방향성 고민 (json, coreData) 이렇게 많은 데이터를 담을 경우가 아닌 경우에 굳이 UUID를 채택하여 하는 부분이 낭비가 아닐까 하는 고민을 했습니다! 프로토콜들에 대한 채택은 inherit(상속)보다는 conform(준수)이라고 표기하는게 낫지 않을까요? 작성 툴은 어떤 것을 쓰신걸까요? (draw.io같은데 맞을까요?) |
action을 외부에서 주입받도록 변경
There was a problem hiding this comment.
안녕하세요 @kimbs5899 ! 리뷰 반영하시느라 고생하셨습니다~ 👍👍👍
현재 연락처 추가화면쪽이 안보이는데 확인 부탁드릴게요!
해당 부분만 수정되면 머지할 수 있을 것 같습니다.
| @@ -27,30 +22,24 @@ extension ContactListViewController { | |||
|
|
|||
| override func viewDidLoad() { | |||
| super.viewDidLoad() | |||
| contactListView.tableView.delegate = self | |||
| override func viewDidLoad() { | ||
| super.viewDidLoad() | ||
| override func loadView() { | ||
| super.loadView() |
There was a problem hiding this comment.
super.loadView()를 써주셨네요! 해당 구문은 꼭 필요한걸까요?? loadView 공식문서를 한번 살펴봐주시면 좋을 것 같습니다!
| let isTextFieldValidResultDictionary = isTextFieldValid(name: name, age: age, phoneNumber: phoneNumber) | ||
| checkNameTextFieldIsEmpty(name: name) ? showMessageAlertWithActions(inputMessage: CustomAlertString.isEmptyNameAlertMessage.description, actionList: [okAction]) : isValidCheckAndShowAlert(inputValidDictionary: isTextFieldValidResultDictionary,contactInfoName: name, contactInfoAge: Int(age) ?? 0, contactInfoPhoneNumber: phoneNumber) |
There was a problem hiding this comment.
분리 잘 해주셨습니다. 다만 조금 더 분리해볼 수 있을 것 같아요!
더불어 삼항연산자를 써주셨는데 한줄이 너무 길어지는 것 같아요. 다른 방법으로 하면 어떨까요?
| return formatTextVaildCheck(formatText: formatText) | ||
| } | ||
|
|
||
| private func formatTextVaildCheck(formatText: String) -> String { |
There was a problem hiding this comment.
유효성 검사 로직은 없는 것 같은데 ValidCheck라는 네이밍이 들어간 이유가 있을까요??
| override init(frame: CGRect) { | ||
|
|
||
| nameLabel = UILabel() | ||
| ageLabel = UILabel() | ||
| phoneNumberLabel = UILabel() | ||
|
|
||
| nameTextField = UITextField() | ||
| ageTextField = UITextField() | ||
| phoneNumberTextField = UITextField() | ||
|
|
||
| nameTextFieldView = UIStackView() | ||
| ageTextFieldView = UIStackView() | ||
| phoneNumberTextFieldView = UIStackView() | ||
| stackView = UIStackView() | ||
|
|
||
| super.init(frame: frame) | ||
|
|
||
| setupLabel() | ||
| setupTextField() | ||
| setupTextFieldView() | ||
| setupStackView() | ||
|
|
||
| setupUI() | ||
| setupSubView() | ||
| setupKeyboard() | ||
| setupConstraints() |
| nameLabel = configureLabel(with: CustomValidString.nameText.description) | ||
| ageLabel = configureLabel(with: CustomValidString.ageText.description) | ||
| phoneNumberLabel = configureLabel(with: CustomValidString.phoneNumberText.description) |
There was a problem hiding this comment.
세 label 인스턴스는 init에서 이미 생성해주었는데 여기서 새로 생성해서 재할당을 해주네요! 이렇게 만드신 이유가 있을까요? 🤔
| if label == nameLabel { | ||
| nameTextFieldView = stack | ||
| } else if label == ageLabel { | ||
| ageTextFieldView = stack | ||
| } else if label == phoneNumberLabel { | ||
| phoneNumberTextFieldView = stack |
There was a problem hiding this comment.
여기도 프로퍼티에 이미 StackView 인스턴스가 초기화되어있는데 재할당을 해주고 계시네요! 🤔 확인 부탁드릴게요~
| stackView.translatesAutoresizingMaskIntoConstraints = false | ||
| } | ||
|
|
||
| private func setupConstraints() { |
There was a problem hiding this comment.
연락처 추가화면쪽에 뷰가 제대로 표시되지 않는 것 같아요~! 확인 부탁드릴게요!
There was a problem hiding this comment.
@kimbs5899 고생하셨습니다!
라인별로 코멘트를 추가적으로 남겼습니다. 마지막으로 확인 부탁드릴게요.
더불어 아래의 사항들 확인해보시면 좋을 것 같습니다.
- 앱 시작 후 연락처 추가 시 'Save'를 누르면 앱 crash
- 연락처에 12자리 이상 입력하면 '-'이 다 사라짐
개발 후에 동작여부를 꼼꼼히 체크하는 것은 반드시 필요한 과정이라는 점, 알아주시면 좋을 것 같아요~! 😄
3주동안 고생 많으셨습니다!
| let isTextFieldValidResultDictionary = isTextFieldValid(inputContact: decodeContact(input: textList)) | ||
| checkNameTextFieldIsEmpty(name: name) ? showMessageAlertWithActions(inputMessage: CustomAlertString.isEmptyNameAlertMessage.description, actionList: [okAction]) : isValidCheck(inputValidDictionary: isTextFieldValidResultDictionary,contactInfo: decodeContact(input: textList)) |
There was a problem hiding this comment.
지금도 충분히 잘 분리해주셨는데요! 제가 생각했던 코드는 아래와 같습니다. 한번 참고해보시면 좋을 것 같아요!
@objc
private func saveButtonTapped() {
let name: String = detailView.nameTextField.text?.replacingOccurrences(of: " ", with: "") ?? ""
let age: String = detailView.ageTextField.text ?? ""
let phoneNumber: String = detailView.phoneNumberTextField.text ?? ""
let isNameValid = isNameValid(name)
let isAgeValid = isAgeValid(age)
let isPhoneNumberValid = isPhoneNumberValid(phoneNumber)
if isNameValid && isAgeValid && isPhoneNumberValid {
showAllFieldIsValid()
createNewContactItem()
moveToContactList()
} else {
let validationReport = createValidationCheckReport(
isNameValid: isNameValid,
isAgeValid: isAgeValid,
isPhoneNumberValid: isPhoneNumberValid
)
showFieldIsInvalid(from: validationReport)
}
}
private func isNameValid(_ name: String) -> Bool {
// name 유효성 검사
}
private func isAgeValid(_ age: String) -> Bool {
// age 유효성 검사
}
private func isPhoneNumberValid(_ phoneNumber: String) -> Bool {
// phone number 유효성 검사
}
private func createValidationCheckReport(
isNameValid: isNameValid,
isAgeValid: isAgeValid,
isPhoneNumberValid: isPhoneNumberValid
) -> InvalidField {
enum InvalidField {
case name
case age
case phone
}
// 세가지 조건을 비교하여 위 3개의 케이스 중 하나 리턴
}| @@ -76,34 +77,39 @@ extension DetailContactViewController { | |||
| } | |||
|
|
|||
| private func checkNameTextFieldIsEmpty(name: String) -> Bool { | |||
There was a problem hiding this comment.
Bool을 반환하는 함수는 is/shoud/need 등~한가?라는 이름으로 떨어지게 지어주시는게 좋을 것 같아요~
| return name.isEmpty ? true : false | ||
| } | ||
|
|
||
| private func decodeContact(input: [String]) -> Contact{ |
There was a problem hiding this comment.
decode라는 이름은 '데이터를 디코딩한다' 라는 뜻으로 이해할 수도 있어서 여기서는 다른 네이밍으로 가져가는게 좋을 것 같습니다.
만든다, 생성한다 정도의 이름이면 좋을 것 같습니다.
| let namePattern: String = CustomValidString.RegularExpressionNamePatthenText.description | ||
| let agePattern: String = CustomValidString.RegularExpressionAgePatthenText.description | ||
| let phoneNumberPattern: String = CustomValidString.RegularExpressionPhoneNumberPatthenText.description | ||
| private func isTextFieldValid(inputContact: Contact) -> [String:Bool] { |
There was a problem hiding this comment.
이 함수는 Bool을 반환하는 함수가 아니니 다른 네이밍을 해주는게 좋을 것 같네요!
|
|
||
| return [CustomValidString.validDictionaryNameKeyText.description: isNameValidResult, | ||
| CustomValidString.validDictionaryAgeKeyText.description: isAgeValidResult, | ||
| CustomValidString.validDictionaryPhoneNumberKeyText.description: isPhoneNumberValidResult] | ||
| } | ||
|
|
||
| private func isValidCheckAndShowAlert(inputValidDictionary: [String: Bool], contactInfoName: String, contactInfoAge: Int, contactInfoPhoneNumber: String) { | ||
| private func isValidCheck(inputValidDictionary: [String: Bool], contactInfo: Contact) { |
There was a problem hiding this comment.
isValid와 check라는 동사가 들어가서 조금 헷갈리는데요.
checkValidation 정도면 어떨까요?
| private lazy var nameLabel: UILabel = setupLabel(text: CustomTextFieldString.nameText.description) | ||
| private lazy var ageLabel: UILabel = setupLabel(text: CustomTextFieldString.ageText.description) | ||
| private lazy var phoneNumberLabel: UILabel = setupLabel(text: CustomTextFieldString.phoneNumberText.description) | ||
|
|
||
| var nameTextField: UITextField | ||
| var ageTextField: UITextField | ||
| var phoneNumberTextField: UITextField | ||
| lazy var nameTextField: UITextField = setupTextField() | ||
| lazy var ageTextField: UITextField = setupTextField() | ||
| lazy var phoneNumberTextField: UITextField = setupTextField() | ||
|
|
||
| var nameTextFieldView: UIStackView | ||
| var ageTextFieldView: UIStackView | ||
| var phoneNumberTextFieldView: UIStackView | ||
| lazy var nameTextFieldView: UIStackView = setupTextFieldView(label: nameLabel, textField: nameTextField) | ||
| lazy var ageTextFieldView: UIStackView = setupTextFieldView(label: ageLabel, textField: ageTextField) | ||
| lazy var phoneNumberTextFieldView: UIStackView = setupTextFieldView(label: phoneNumberLabel, textField: phoneNumberTextField) | ||
|
|
||
| private var stackView: UIStackView | ||
| private lazy var stackView: UIStackView = setupStackView() |
There was a problem hiding this comment.
변수를 lazy로 만들어서 초기화시킬수도 있지만 함수 자체를 static 함수로 만들어서 처음부터 접근 가능하게 만들 수도 있을 것 같네요. 나중에 한번 시도해보세요!
| } | ||
|
|
||
| private func configureLabel(with text: String) -> UILabel { | ||
| private func setupLabel(text: String) -> UILabel { |
There was a problem hiding this comment.
setup도 좋지만 여기서는 UILabel을 만들어서 return하므로 -> create/generate 정도의 이름이 좋을 것 같습니다
| nameTextFieldView = configureTextFieldView(with: nameLabel, textField: nameTextField) | ||
| ageTextFieldView = configureTextFieldView(with: ageLabel, textField: ageTextField) | ||
| phoneNumberTextFieldView = configureTextFieldView(with: phoneNumberLabel, textField: phoneNumberTextField) | ||
| private func setupPhoneNumerAddTarget(_ textField: UITextField) { |
| let stack: UIStackView = UIStackView(arrangedSubviews: [label, textField]) | ||
| stack.axis = .horizontal | ||
| stack.distribution = .fillProportionally | ||
| stack.alignment = .fill | ||
| stack.spacing = 14 | ||
|
|
||
| if label == nameLabel { | ||
| nameTextFieldView = stack | ||
| } else if label == ageLabel { | ||
| ageTextFieldView = stack | ||
| } else if label == phoneNumberLabel { | ||
| phoneNumberTextFieldView = stack | ||
| } | ||
|
|
||
| stack.addSubview(label) | ||
| stack.addSubview(textField) | ||
| return stack | ||
| } | ||
|
|
||
| private func setupStackView() { | ||
| stackView = configureStackView() | ||
| } | ||
|
|
||
| private func configureStackView() -> UIStackView { | ||
| private func setupStackView() -> UIStackView { | ||
| let stack: UIStackView = UIStackView(arrangedSubviews: [nameTextFieldView, ageTextFieldView, phoneNumberTextFieldView]) | ||
| stack.axis = .vertical | ||
| stack.distribution = .fillEqually | ||
| stack.alignment = .fill | ||
| stack.spacing = 14 | ||
| stack.addSubview(nameTextFieldView) | ||
| stack.addSubview(ageTextFieldView) | ||
| stack.addSubview(phoneNumberTextFieldView) | ||
| return stack |
There was a problem hiding this comment.
UIStackview를 초기화할 때 arrangedSubviews로 하위뷰들을 추가하셨는데 아래에서 또 추가해주신 이유가 있을까요?
| nameLabel.translatesAutoresizingMaskIntoConstraints = false | ||
| nameTextField.translatesAutoresizingMaskIntoConstraints = false | ||
| nameTextFieldView.translatesAutoresizingMaskIntoConstraints = false | ||
| ageLabel.translatesAutoresizingMaskIntoConstraints = false | ||
| ageTextField.translatesAutoresizingMaskIntoConstraints = false | ||
| ageTextFieldView.translatesAutoresizingMaskIntoConstraints = false | ||
| phoneNumberLabel.translatesAutoresizingMaskIntoConstraints = false | ||
| phoneNumberTextField.translatesAutoresizingMaskIntoConstraints = false | ||
| phoneNumberTextFieldView.translatesAutoresizingMaskIntoConstraints = false | ||
| stackView.translatesAutoresizingMaskIntoConstraints = false |
There was a problem hiding this comment.
stackview 내부에 들어가는 요소들도 translatesAutoresizingMaskIntoConstraints 설정이 필요할까요?
연락처 관리 앱 프로젝트 [STEP 2]
멤버 : @kimbs5899, @tawans
리뷰어 : @ictechgy
연락처 관리 앱 프로젝트의 두번째 Step입니다!
정리하여 PR 드립니다. 감사합니다!
이번 스텝을 통해서 배운 점
Data 전달방식
ContactManager
formatPhoneNumber
구성
Model
Contact.swift
ContactListManager.swift
View
ContectListView.swift
DetailContectView.swift
Controller
ContactListViewController.swift
DetailContactViewController.swift
Support
String+
UIViewController+
CustomString
질문
UML