연락처 관리앱 [Step3] 미르, 루피 #82
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
셀을 선택했을때 짧게 회색으로 변하도록 구현
* docs: 기본파일 생성 * feat: tableView layout 코드 추가 * feat: gitignore파일 추가 * feat: JSON 데이터형식 파일을 올리고 디코딩하여 UI에 연락처들을 구현 * feat: tabelView 상세 설정 코드 추가 및 cell identifier 등록 * feat: 셀에 accessory에 disclosureIndicator 적용 셀을 선택했을때 짧게 회색으로 변하도록 구현 * feat: 연락처 관리 기능 추가 * refactor: 띄어쓰기, 오타 수정 * refactor: AppDelegate 주석제거 * refacot: ViewController 네이밍 수정, final키워드 * refactor: SceneDelegate 주석제거, 개행수정 * refacotr: 연산프로퍼티 네이밍 수정 * refactor: 테이블뷰 네이밍 수정, 테이블뷰 프로퍼티 초기화시점에 최가화할수 있는 속성으로 수정 * refactor: cell identifier 형식 변경 및 delegate & datasource markdown 추가 * refactor: json데이터를 파싱하는 타입을 모델에 분리, 에러처리를 알럿창으로 띄우도록 구현 * refactor: UUID를 통해 기존의 정보를 수정하면 업데이가 되도록 수정 --------- Co-authored-by: mireu <[email protected]>
cell을 새로 정의하여 레이아웃 수정
FirstDo
reviewed
Jan 19, 2024
ios-contact-manager-ui/ios-contact-manager-ui/Sources/Controllers/ContactViewController.swift
Show resolved
Hide resolved
Comment on lines
50
to
53
| view.backgroundColor = .white | ||
| layout() | ||
| parse() | ||
| setupNaviBar() |
There was a problem hiding this comment.
함수 추상화 수준도 맞춰주면 좋을것 같습니다
layout
parse
setupNaviBar 는 다 매서드 인데
view.backgroundColor = .white 혼자 어색해보이네요!
통일성있는코드 깔끔한 코드를 작성하는게 추후 사전과제에서도 더 좋은 인상을 줄 수 있을겁니다 ㅎㅎ
There was a problem hiding this comment.
추상화 수준을 맞춰주는것은 가독성에 도움을 줍니다
그리고 딱히 정해진 규칙이 있는것은 아니지만
viewBackground, setNaviBar 이런식으로 일일히 다 나누기보다는 setup 함수 하나에서 하는게 좋을것 같네요
주석 & 개행정도의 구분이면 충분할것 같아요
fucn setup() {
// setup Navigatoin
title = "맛있는 코드"
// setup TableView
tableview.delegate = self
tableview.datasource = self
}
Comment on lines
58
to
61
| NSLayoutConstraint.activate([tableView.topAnchor.constraint(equalTo: self.view.topAnchor), | ||
| tableView.leadingAnchor.constraint(equalTo: self.view.leadingAnchor), | ||
| tableView.trailingAnchor.constraint(equalTo: self.view.trailingAnchor), | ||
| tableView.bottomAnchor.constraint(equalTo: self.view.safeAreaLayoutGuide.bottomAnchor)]) |
There was a problem hiding this comment.
이친구도 마찬가지인게,
NSLayoutConstraint.activate 을 사용한곳마다 내려쓰기 방식이 다 다릅니다
한가지로 통일시켜주세요!
There was a problem hiding this comment.
통일성있게 코드를 작성하는것도 중요합니다
이런 컨벤션을 맞추기 위한 툴로는 swiftLint, swiftFormat 등이 있습니다!
| navigationController?.navigationBar.backgroundColor = .white | ||
| navigationController?.navigationBar.shadowImage = UIImage() | ||
|
|
||
| self.navigationItem.rightBarButtonItem = self.plusButton |
There was a problem hiding this comment.
self 도 마찬가지입니다
특별한 이유가 없다면, 모두 붙이거나 or 모두 안붙이거나 하나로 통일하는게 좋습니다
There was a problem hiding this comment.
마찬가지로
이런 컨벤션을 맞추기 위한 툴로는 swiftLint, swiftFormat 등이 있습니다!
| present(alert, animated: true) | ||
| } | ||
| } | ||
| @objc func plusButtonTapped() { |
There was a problem hiding this comment.
역시 마찬가지로
공부할때는 켄변션을 통일하는 습관을 들이고
나중에는 swiftLint, swiftFormat 등의 툴을 쓰면 편합니다!
Comment on lines
105
to
110
| if let name = search.searchBar.text, !name.isEmpty { | ||
| let contact = contactManager.contacts.filter { $0.name.contains(name) } | ||
| cell.contact = contact[indexPath.row] | ||
| } else { | ||
| cell.contact = contactManager.contacts[indexPath.row] | ||
| } |
There was a problem hiding this comment.
이부분도 좋습니다.
다만 이방식도 절대 틀린건 아니지만?
지금 cellForRow에서 매번 전체 배열 filter를 돌리고 있는데
cellForRow의 경우 Cell하나당 호출되는매서드니까.. 굉장히 비효율적이겠죠?
데이터가 많아진다면 문제가 생길여지가 있습니다
배열을 하나 더 만들고, 검색결과를 해당 배열에 넣고, 쿼리 있을때는 이 배열을 보여주는 식으로 하는 방식도 좋습니다.
연산프로퍼티로 만들면 더 좋겠네요!
이부분 설명이 조금 부족했다면 dm 부탁드려요 ㅎㅎ
ex)
var filterdContacts: [Contact] {
// - TODO
}There was a problem hiding this comment.
연산프로퍼티를 사용해서 indexPath에 접근하도록 해당커밋에서 수정해봤는데 의도하신바가 맞는지는 잘모르겠네요..확인부탁드립니다!
There was a problem hiding this comment.
의도한 바가 맞고, 제 설명이 조금 부족했는데 너무 완벽하게 해주셨습니다 ㅎㅎ
이전보다 훨씬 효율적인 코드가 된것같네요
사전과제를 전 코드처럼 작성한다면 해당부분에서 공격이 들어올 가능성이 커 보입니다.
CellForRow가 언제 호출되는지 아는지?
안다면 왜 거기서 전체배열 filter를 돌리는지??
Comment on lines
124
to
131
| func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { | ||
| tableView.deselectRow(at: indexPath, animated: true) | ||
|
|
||
| let detailVC = ContactDetailViewController() | ||
| detailVC.contact = contactManager.contacts[indexPath.row] | ||
| detailVC.delegate = self | ||
| present(UINavigationController(rootViewController: detailVC), animated: true) | ||
| } |
There was a problem hiding this comment.
disclosure Indicator까지 있는 cell을 누르면 보통 navigationPush 방식을 기대할것 같습니다
혹시 저만그런가요? ㅋㅋㅋ
There was a problem hiding this comment.
처음엔 그렇게 못느꼈는데 말씀해주시니 뭔가 push가 적절해보이는거같더라구요 ㅋ
그래서 해당커밋에서 push방식으로 화면전환을 수정해봤습니다!
Comment on lines
81
to
89
| if var newEdit = contact { | ||
| newEdit.phoneNumber = phone | ||
| newEdit.age = age | ||
| newEdit.name = name | ||
| delegate?.update(contact: newEdit) | ||
| } else { | ||
| let newAdd = Contact(name: name, phoneNumber: phone, age: age) | ||
| delegate?.add(contact: newAdd) | ||
| } |
There was a problem hiding this comment.
그쵸 저도 if var 어색하다고 생각합니다
물론 꼭 써야하는 상황이 있을수도 있는데, 지금은 아닌거 같아요
결국 id가 중요한거잖아요?
저라면 아래처럼 할것 같습니다
- 생성자에서 id를 받을수 있게 변경 (업데이트를 위함)
struct Contact: Decodable {
var id: UUID
var name: String
var phoneNumber: String
var age: String
init(id: UUID = UUID(), name: Stting, phoneNumber, age) {
}
}- 코드를 아래와 같이 변경
if let id = contact.id {
delegate?.update(contact: Contact(id: id, name: name, phoneNumber: phone, age: age))
} else {
delegate?.add(contact: Contact(name: name, phoneNumber: phone, age: age))
}방법은 더 다양하게 있을 수 있겠네요!
Comment on lines
101
to
107
| func makeInfo() throws -> (String, String, String) { | ||
| guard let name = detailView.nameTextField.text, Verification.setName(name) else { throw ContactError.errorName } | ||
| guard let age = detailView.ageTextField.text, Verification.setAge(age) else { throw ContactError.errorAge } | ||
| guard let phone = detailView.phoneNumberTextField.text, Verification.setNumber(phone) else { throw ContactError.errorNumber } | ||
|
|
||
| return (name.removeBlank, age, phone) | ||
| } |
There was a problem hiding this comment.
이건 시간이 남으신다면 해보세요
현재는 이렇게 에러를 순서대로 던지기때문에
name, age, phone이 모두 잘못되어도 name이 잘못되었다는 메시지만 뜹니다.
셋다 뜨게할 수는없을까요??
There was a problem hiding this comment.
말씀주신 의도인지는 맞는지 모르겠지만 정보모두를 잘못적었을때 입력이 올바르지 않다라는 알럿창을 띄우도록 해당커밋에서 수정해봤습니다.
There was a problem hiding this comment.
시도해 보신점 좋습니다!
의도는 아래와 같았는데 제 설명이 조금 부족했네요!
ex) 이름, 나이를 잘못입력하고 전화번호를 제대로 입력했을떄
-> 이름 & 나이가 올바르지 않습니다
Comment on lines
+115
to
+118
| func tableView(_ tableView: UITableView, commit editingStyle: UITableViewCell.EditingStyle, forRowAt indexPath: IndexPath) { | ||
| contactManager.delete(index: indexPath) | ||
| tableView.deleteRows(at: [indexPath], with: .fade) | ||
| } |
There was a problem hiding this comment.
현재 cell을 삭제하면 오토레이아웃 에러가 발생하네요!
Unable to simultaneously satisfy constraints.
Probably at least one of the constraints in the following list is one you don't want.
Try this:
(1) look at each constraint and try to figure out which you don't expect;
(2) find the code that added the unwanted constraint or constraints and fix it.
(
"<NSLayoutConstraint:0x6000021459f0 UILabel:0x105478540.height >= 20 (active)>",
"<NSLayoutConstraint:0x600002145a90 UILabel:0x105478840.height >= 20 (active)>",
"<NSLayoutConstraint:0x6000021463a0 V:|-(0)-[UIStackView:0x1054776e0] (active, names: '|':ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell' )>",
"<NSLayoutConstraint:0x600002146800 UIStackView:0x1054776e0.bottom == ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell'.bottom (active)>",
"<NSLayoutConstraint:0x600002146da0 'UISV-canvas-connection' UIStackView:0x1054776e0.top == UILabel:0x105477bd0.top (active)>",
"<NSLayoutConstraint:0x600002146df0 'UISV-canvas-connection' V:[UILabel:0x105478840]-(0)-| (active, names: '|':UIStackView:0x1054776e0 )>",
"<NSLayoutConstraint:0x600002146e40 'UISV-spacing' V:[UILabel:0x105477bd0]-(0.333333)-[UILabel:0x105478540] (active)>",
"<NSLayoutConstraint:0x600002146e90 'UISV-spacing' V:[UILabel:0x105478540]-(0.333333)-[UILabel:0x105478840] (active)>",
"<NSLayoutConstraint:0x6000021472a0 'UIView-Encapsulated-Layout-Height' ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell'.height == 0 (active)>"
)
Will attempt to recover by breaking constraint
<NSLayoutConstraint:0x600002145a90 UILabel:0x105478840.height >= 20 (active)>
Make a symbolic breakpoint at UIViewAlertForUnsatisfiableConstraints to catch this in the debugger.
The methods in the UIConstraintBasedLayoutDebugging category on UIView listed in <UIKitCore/UIView.h> may also be helpful.
Unable to simultaneously satisfy constraints.
Probably at least one of the constraints in the following list is one you don't want.
Try this:
(1) look at each constraint and try to figure out which you don't expect;
(2) find the code that added the unwanted constraint or constraints and fix it.
(
"<NSLayoutConstraint:0x6000021463f0 UILabel:0x105477bd0.height >= 20 (active)>",
"<NSLayoutConstraint:0x6000021459f0 UILabel:0x105478540.height >= 20 (active)>",
"<NSLayoutConstraint:0x6000021463a0 V:|-(0)-[UIStackView:0x1054776e0] (active, names: '|':ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell' )>",
"<NSLayoutConstraint:0x600002146800 UIStackView:0x1054776e0.bottom == ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell'.bottom (active)>",
"<NSLayoutConstraint:0x600002146da0 'UISV-canvas-connection' UIStackView:0x1054776e0.top == UILabel:0x105477bd0.top (active)>",
"<NSLayoutConstraint:0x600002146df0 'UISV-canvas-connection' V:[UILabel:0x105478840]-(0)-| (active, names: '|':UIStackView:0x1054776e0 )>",
"<NSLayoutConstraint:0x600002146e40 'UISV-spacing' V:[UILabel:0x105477bd0]-(0.333333)-[UILabel:0x105478540] (active)>",
"<NSLayoutConstraint:0x600002146e90 'UISV-spacing' V:[UILabel:0x105478540]-(0.333333)-[UILabel:0x105478840] (active)>",
"<NSLayoutConstraint:0x6000021472a0 'UIView-Encapsulated-Layout-Height' ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell'.height == 0 (active)>"
)
Will attempt to recover by breaking constraint
<NSLayoutConstraint:0x6000021459f0 UILabel:0x105478540.height >= 20 (active)>
Make a symbolic breakpoint at UIViewAlertForUnsatisfiableConstraints to catch this in the debugger.
The methods in the UIConstraintBasedLayoutDebugging category on UIView listed in <UIKitCore/UIView.h> may also be helpful.
Unable to simultaneously satisfy constraints.
Probably at least one of the constraints in the following list is one you don't want.
Try this:
(1) look at each constraint and try to figure out which you don't expect;
(2) find the code that added the unwanted constraint or constraints and fix it.
(
"<NSLayoutConstraint:0x6000021463f0 UILabel:0x105477bd0.height >= 20 (active)>",
"<NSLayoutConstraint:0x6000021463a0 V:|-(0)-[UIStackView:0x1054776e0] (active, names: '|':ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell' )>",
"<NSLayoutConstraint:0x600002146800 UIStackView:0x1054776e0.bottom == ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell'.bottom (active)>",
"<NSLayoutConstraint:0x600002146da0 'UISV-canvas-connection' UIStackView:0x1054776e0.top == UILabel:0x105477bd0.top (active)>",
"<NSLayoutConstraint:0x600002146df0 'UISV-canvas-connection' V:[UILabel:0x105478840]-(0)-| (active, names: '|':UIStackView:0x1054776e0 )>",
"<NSLayoutConstraint:0x600002146e40 'UISV-spacing' V:[UILabel:0x105477bd0]-(0.333333)-[UILabel:0x105478540] (active)>",
"<NSLayoutConstraint:0x600002146e90 'UISV-spacing' V:[UILabel:0x105478540]-(0.333333)-[UILabel:0x105478840] (active)>",
"<NSLayoutConstraint:0x6000021472a0 'UIView-Encapsulated-Layout-Height' ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell'.height == 0 (active)>"
)
Will attempt to recover by breaking constraint
<NSLayoutConstraint:0x6000021463f0 UILabel:0x105477bd0.height >= 20 (active)>
Make a symbolic breakpoint at UIViewAlertForUnsatisfiableConstraints to catch this in the debugger.
The methods in the UIConstraintBasedLayoutDebugging category on UIView listed in <UIKitCore/UIView.h> may also be helpful.
Unable to simultaneously satisfy constraints.
Probably at least one of the constraints in the following list is one you don't want.
Try this:
(1) look at each constraint and try to figure out which you don't expect;
(2) find the code that added the unwanted constraint or constraints and fix it.
(
"<NSLayoutConstraint:0x6000021463a0 V:|-(0)-[UIStackView:0x1054776e0] (active, names: '|':ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell' )>",
"<NSLayoutConstraint:0x600002146800 UIStackView:0x1054776e0.bottom == ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell'.bottom (active)>",
"<NSLayoutConstraint:0x600002146da0 'UISV-canvas-connection' UIStackView:0x1054776e0.top == UILabel:0x105477bd0.top (active)>",
"<NSLayoutConstraint:0x600002146df0 'UISV-canvas-connection' V:[UILabel:0x105478840]-(0)-| (active, names: '|':UIStackView:0x1054776e0 )>",
"<NSLayoutConstraint:0x600002146e40 'UISV-spacing' V:[UILabel:0x105477bd0]-(0.333333)-[UILabel:0x105478540] (active)>",
"<NSLayoutConstraint:0x600002146e90 'UISV-spacing' V:[UILabel:0x105478540]-(0.333333)-[UILabel:0x105478840] (active)>",
"<NSLayoutConstraint:0x6000021472a0 'UIView-Encapsulated-Layout-Height' ios_contact_manager_ui.ContactDetailCell:0x107066000'ContactDetailCell'.height == 0 (active)>"
)
Will attempt to recover by breaking constraint
<NSLayoutConstraint:0x600002146e90 'UISV-spacing' V:[UILabel:0x105478540]-(0.333333)-[UILabel:0x105478840] (active)>
Make a symbolic breakpoint at UIViewAlertForUnsatisfiableConstraints to catch this in the debugger.
The methods in the UIConstraintBasedLayoutDebugging category on UIView listed in <UIKitCore/UIView.h> may also be helpful.
FirstDo
reviewed
Jan 19, 2024
Comment on lines
+35
to
+37
| let customBtton = UIButton(type: .system) | ||
| customBtton.setTitle("Save", for: .normal) | ||
| customBtton.addTarget(self, action: #selector(saveButtonTapped), for: .touchUpInside) |
There was a problem hiding this comment.
추가사항
현재는 변경사항이 없어도 Save 버튼이 활성화 되어있어요.
변경점이 있을때만 Save버튼이 활성화되게끔 하는건 어떨까요
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
안녕하세요~ @FirstDo
미르, 루피 입니다! 연락처 관리 앱 STEP3 PR 보냅니다!
이번에도 PR 잘 부탁드립니다~😀
프로젝트 구현 요구사항
STEP 3 - 연락처 삭제 / Cell Layout / Dynamic Type
Bonus STEP
해외 연락처 입력
연락처 정보 변경
🤔고민이 되었던 점
기존의 연락처에서 정보를 들어가면 기존의 정보를 어떻게 수정해줄지에 대해 고민을 했었습니다
let new = Contact(name: name, phoneNumber: phone, age: age)로 id가 추가되는ContactManager의update메서드를 타지 못하고, 연락처가 추가가 되었습니다. 그래서contact프로퍼티가 nil이면Contact를 추가하고, nil이 아니면contact의 기존의 연락처를 받도록 삼항연산자를 써줬습니다.Dynamic Type을 코드로 적용하는 방법에 대한 고민을 했었습니다.
adjustsFontForContentSizeCategory메서드를 true로 지정을 해줬습니다.NavigationBar에는 다이나믹타입을 적용하지 않았는데 알아서 Dynamic Type이 적용되어titleTextAttributes를 통해 고정값을 넣어줬습니다. barButton의 경우는 UIButton을 커스텀으로 만들어 사이즈를 적용하고 UIBarButtonItem에 커스텀버튼을 넣어서 DynamicType을 적용하지 않도록 해줬습니다.국제전화번호 입력 형식과 정규식 표현에 대해 고민했습니다.
국제전화번호의 입력형식과 정규식 표현은 까다로웠습니다. 기존의 지역번호와 휴대폰 번호, 그리고 추가된 국제전화번호의 입력 형식을 구분하기 위해 총 4가지의 경우의 수로 조건을 나누었습니다.
else첫 4글자가 "+820" 인 경우else if첫 3글자가 "+82" 인 경우else if첫글자가 "0"이거나 "02"인 경우else그 외의 경우이에 따른 정규식 표현은 다음과 같이 해주었습니다.
새 연락처 입력 화면과 기존 연락처 수정 화면의 구분에 대해 고민했습니다.
새 연락처 입력 화면과 기존 연락처 수정 화면은 같은 베이스의 view를 가지고 있고, title이나 textField값만 다르기 때문에 두 화면에 접근 시 가지고 가는 contact의 nil 유무에 따라 text를 다르게 해주었습니다.
새 연락처 입력 화면 접근 시 contact값이 전달되지 않지만, 기존 연락처 수정 화면 접근시 contact값이 전달됩니다.
따라서 contact의 nil 유무에 따라 값을 다르게 처리해주는 방식을 선택했습니다.
조언을 구하고 싶은 부분
let new = Contact(name: name, phoneNumber: phone, age: age)로 id가 추가되는ContactManager의update메서드를 타지 못하고, 연락처가 추가가 되었습니다. 그래서contact프로퍼티가 nil이면Contact를 추가하고, nil이 아니면contact의 기존의 연락처를 받도록 삼항연산자를 써줬었는데 코드를 좀더 가독성있게 contact프로퍼티를 옵셔널 바인딩을 해줘서 update파라미터에 넣어줬습니다. 그러나, if let 옵셔널 바인딩을 썼더니, 그림과 같이 let이 아닌 var를 써줘라고 에러가 나서 수정을 해줬습니다만, if var가 조금 어색해보여 삼항연산자, if var, guard var가 아닌 다른 방법으로 값을 구할수 있는지 조언을 얻고 싶습니다.