Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Step Bonus] 연락처관리프로그램 UI ― iyeah, Jenna #17

Open
wants to merge 11 commits into
base: 1_Jenna
Choose a base branch
from

Conversation

ueunli
Copy link

@ueunli ueunli commented Feb 9, 2023

안녕하세요 Cory 🐶 !! @corykim0829
코리의 빠르고 세세한 리뷰 덕분에 보너스 스텝까지 도전해 볼 수 있게 되었어요 감사합니다 🍀
2주 동안 고생하셨고, 마지막 스텝도 리뷰 잘 부탁드려요 🫶🏻
 

✅ 구현 사항

  • 테이블뷰 셀 편집
  • 사용자정의 테이블뷰 셀 구현
  • 테이블뷰 검색기능 구현
  • Dynamic Type 적용
삭제기능 검색기능

 

✅ 구현 과정

1️⃣ 테이블뷰 셀 편집

Swipe로 데이터 및 해당 셀을 Delete하기

사용한 방법

  • UITableViewDataSource 프로토콜 내에 있는 tableView(_:commit:forRowAt:) 메서드의 editingStyle.delete 로 해당 셀의 연락처를 삭제되도록 구현했어요.

트러블 슈팅

  • 검색결과 화면에서도 올바른 셀이 삭제되도록 하기
    • 삼항연산자를 활용하여, 아래 구현사항 트러블슈팅에서 말씀드린 isSearching에 따라 index로 접근할 프로필 배열이 profiles / filteredProfiles 중 어느쪽인지 결정하는 로직을 추가했어요.
  • 동명이인이 있어도 정확히 삭제되도록 하기
    • [1] 초반에 profiles에서 name이 일치하는 결과를 가져오려고 했지만, 동명이인이 대신 삭제되는 문제가 발생했어요.
    • [2] Model인 Profile이 Hashable프로토콜(= 즉 Equatable도 채택함)을 채택했으므로 커스텀 이항연산자를 구현해보려다가 아래와 같은 발상이 떠올라 보류했어요.
    • [3] tableView(_:cellForRowAt:)메서드에서 indexPath.row로 profile을 불러왔으므로, 역으로 해당 index의 profile을 꺼내어 삭제하면 해당 셀의 profile이 삭제될 거라고 생각하고 구현했어요.

 

2️⃣ 테이블뷰 검색기능 구현

이름(name) 기준으로 검색하기

사용한 방법

  • 뷰컨의 navigationItem에 UISearchBar인스턴스를 넣는 방법과 UISearchController인스턴스를 넣는 방법 중 후자를 사용했어요.
  • 새로운 변수 2개를 추가로 선언했어요
    • filteredProfiles: 검색이 시작되기 전에는 불필요하므로 lazy로 선언
    • isSearching: 서치바가 isActive고 검색란이 비어있지 않다면 true인 계산속성

트러블 슈팅

  • 전화번호부 목록에 동명이인이 있다면 나이순으로 정렬되도록 처리했어요.
  • 이름에 대소문자가 섞여 있을 때 오름차순으로 올바르게 정렬되지 않고 검색이 되지 않는 트러블이 있어, lowercased() 메서드를 이용해 대소문자 구분없이 오름차순으로 정렬되도록 수정했어요.

 
 

ueunli and others added 6 commits February 9, 2023 10:05
- 불필요한 표현(self) 및 접근 삭제
- 대소문자 상관 없이 검색되도록 개선
- 동명이인일 경우에도 정확히 삭제되는지 확인하기 쉬운 더미데이터로 변경
- 동명이인일 경우 나이순으로 정렬되도록 수정
- 아래로 스크롤해도 서치바가 보이도록 처리
- `makeSearchBar()`를 private func로 수정
- extension 내 두 메서드의 순서 교체 (private이 밑으로 오도록)
- `tableView(_:commit:forRowAt:)`메서드의 불필요한 로직 생략
Copy link

@corykim0829 corykim0829 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

보너스 미션까지 두분 정말정말 수고 많으셨습니다 👍

깔끔하게 구현하셔서 크게 피드백할 부분이 없네요! 👏👏👏

삼항 연산자가 편리하긴 하지만, 가독성을 떨어트릴 때도 있으니 항상 사용하실 때에는 주의해서 사용하는게 좋습니다!

2주동안 고생 많으셨습니다!!!

}

extension ListProfileViewController: UISearchResultsUpdating {
func updateSearchResults(for searchController: UISearchController) {
guard let text = searchController.searchBar.text else { return }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guard 문의 else 뒤 return은 개행 처리해주세요. 확실하게 return 되는 부분을 구분하는게 좋습니다

Copy link

@iyeahh iyeahh Feb 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수정했습니다! 앞으로도 개행해서 처리하도록 할게요 🙋🏻‍♀️

해당 커밋 : c0ce3cb

extension ListProfileViewController: UISearchResultsUpdating {
func updateSearchResults(for searchController: UISearchController) {
guard let text = searchController.searchBar.text else { return }
filteredProfiles = profiles.filter { $0.name.lowercased() == text.lowercased() }

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

여기도 { , } 부분 개행 해주세요.
한줄에 너무 많은 코드가 있으면 작성자가 아니라면, 가독성이 떨어집니다.

Copy link

@iyeahh iyeahh Feb 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

가독성을 위해 개행 처리하여 수정했습니다 :)

해당 커밋 : c0ce3cb

return lhs != rhs ? lhs < rhs : $0.age < $1.age
}
}
private lazy var filteredProfiles = [Profile]()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lazy var로 선언한 부분 좋네요! 👍👍
filtered보다는 검색결과 프로필들이라는 느낌의 프로퍼티명으로 바꾸면 더 좋을 것 같아요!

Copy link

@iyeahh iyeahh Feb 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다! profileSearchResults 로 변경하였어요 👍🏻

해당 커밋 : c0ce3cb

Comment on lines 13 to 16
contactManageSystem.profiles.sorted {
let (lhs, rhs) = ($0.name.lowercased(), $1.name.lowercased())
return lhs != rhs ? lhs < rhs : $0.age < $1.age
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

복잡한 데이터 로직은 컨트롤러에서 보이지 않고, 모델 쪽에서 처리하도록 하는게 어떨까요?

Copy link
Author

@ueunli ueunli Feb 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ContactManageSystemsortProfiles()메서드를 만들어 처리하도록 했어요!
훨씬 깔끔해진 것 같아요 감사합니다🙆🏻‍♀️

해당 커밋: e544c0d

private var isSearching: Bool {
let searchBarController = self.navigationItem.searchController
let isActive = searchBarController?.isActive ?? false
let isEmpty = searchBarController?.searchBar.text?.isEmpty ?? false

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isEmpty는 값을 가져오지 못할 때 ??에서 true로 처리해야하지 않을까요? 👀

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞아요🫢 감사합니다 바로 수정해 두었어요ㅠㅠ!!

해당 커밋: c0ce3cb

tableView.reloadData()
}

private func makeSearchBar() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 함수는 UISearchResultsUpdating extension에 선언하기 보다는,
UI 요소를 구현하는 부분을 extension으로 정리하면 좋을 것 같네요.
참고만 하세요! :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setUp요소() 유형의 메서드들을 모아놓으라는 말씀이시죠?!
확실히 그러면 뷰가 복잡해져도 보기에 훨씬 편할 것 같아요..!!!

이번 기회에 저희 코드를 다시 살펴봤는데 해당 메서드를
'searchBar 저장프로퍼티'와 'setUpSearchBar() 메서드'로 분리하고 후자를 다른 UI세팅용 메서드들과 묶어도 좋겠다는 생각이 들었어요.

지금 프로젝트는 그런 메서드가 각 VC에 하나씩 뿐이라 의논 끝에 말씀 주신 것처럼 참고만 하기로 했지만, 기억해 두었다가 이후 프로젝트들에서 잘 활용하도록 할게요🤠 좋은 팁 감사합니다!!

Comment on lines 87 to 89
let profile = isSearching ? filteredProfiles[indexPath.row] : profiles[indexPath.row]
contactManageSystem.remove(profile: profile)
tableView.deleteRows(at: [indexPath], with: .fade)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

깔끔하게 구현 잘하셨네요 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants