[Team-17 iOS 잭슨] 팀17 메인 View와 해당 위치 검색하는 View 구성. #45
Conversation
|
|
||
| let locationIDs = [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25] | ||
| let locationNames = ["강남구", "강동구", "강북구", "강서구", "관악구", "광진구", "구로구", "금천구", "노원구", "도봉구", "동대문구", "동작구", "마포구", "서대문구", "서초구", "성동구", "성북구", "송파구", "양천구", "영등포구", "용산구", "은평구", "종로구", "중구", "중랑구"] | ||
| let locationImageNames = ["CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel","CodeSquadHotel"] |
There was a problem hiding this comment.
AppDelegate에서 변수로 더미데이터를 만드는 것보다 더미 데이터를 관리할 수 있는 객체를 만드는 것은 어떨까요?
There was a problem hiding this comment.
아하 넵! 따로 데이터를 가지고 있는 객체를 만들어보겠습니다!
| } | ||
| } | ||
|
|
||
| print(Realm.Configuration.defaultConfiguration.fileURL!) |
There was a problem hiding this comment.
앗 넵. 제가 RealmDB 파일을 확인해보려다보니 print문으로 경로를 확인하려다보니 써놓았네요. 감사합니다.
| super.viewDidLoad() | ||
| } | ||
|
|
||
| override func viewWillAppear(_ animated: Bool) { |
There was a problem hiding this comment.
super.viewWillAppear(animated) 빠진 이유가 있을까요? 가급적 super 메서드는 호출해주세요!
| } | ||
| @IBAction func searchHotels(_ sender: Any) { | ||
| let vc = self.storyboard?.instantiateViewController(identifier: "SearchHotelsViewController") | ||
| self.navigationController?.pushViewController(vc!, animated: true) |
There was a problem hiding this comment.
- vc! 보다
guard let , if let, 옵셔널 체이닝 등활용해주세요.!앱 크래시가 날 수 있습니다! - 저도 사용하지 않는 화면에서 idenfitier 잘못 입력하고 테스트 하지 않은채 실제 앱스토어 올라갔다가 크래시 비율이 올라간 경험을 한 적이 있습니다. 실수를 방지하는 것이 좋은데 뷰컨트롤러 인스턴스화 할 때 직접 SearchHotelsViewController 적는 것을 방지할 수 있습니다. ViewController 인스턴스화 할 때 다양한 방법으로 extension 할 수 있지만 제네릭 함수를 활용할 수 있습니다:)
extension UIStoryboard {
static func create<T: UIViewController>(identifier: T.Type, name storyboardName: String) -> T {
let identifier = String(describing: T.self)
guard let viewController = UIStoryboard(name: storyboardName, bundle: nil).instantiateViewController(withIdentifier: identifier) as? T else {
fatalError()
}
return viewController
}
}There was a problem hiding this comment.
옵셔널에 대해 기초가 부족하다보니 guard, if let의 사용유무를 잘 모르고 있었습니다.
실제 경험을 얘기해주셔서 더 와닿았습니다. 고치도록 하겠습니다!
| } | ||
|
|
||
| func setNotificationCenter() { | ||
| NotificationCenter.default.addObserver(self, selector: #selector (SearchLocationsViewController.changeCollectionView), name: Notification.Name("cellsChanged"), object: nil) |
There was a problem hiding this comment.
Notification.Name("cellsChanged") add, post 적어도 사용하는 곳이 2군데 일텐데 extension 하거나 따로 상수를 만들어서 관리하는 것이 좋습니다!
There was a problem hiding this comment.
아하! 넵. 확장성을 생각못하구 실행이 되냐에만 초점을 둔 것 같네요. 따로 빼도록 하겠습니다.
There was a problem hiding this comment.
사소한 문제지만, 파일 생성 위치도 어디에다가 할 것인지 생각해볼 여지가 많습니다. 단순하게 Notification.Name으로 extension 한다면 여러 extension 관리하는 그룹에서 생성할 것인지, add, post 하고 있는 도메인 그룹이나 레이어에다가 둘 것인지 코드 한 줄을 작성하거나 파일을 생성하는 위치 등 사소하더라도 다른 사람을 설득할 수 있는 본인만의 근거를 만드는 연습을 하면 좋아요🙂
| if searchLocationsController.isActive { | ||
| return filteredLocations.locations.count | ||
| }else { | ||
| allLocations = dbManager.getLocations() |
There was a problem hiding this comment.
dbManager.getLocations() 가져오는 위치가 적절해보이지 않습니다. SearchLocationDataSource 객체가 생성할 때 allLocations 데이터 미리 셋팅되어 있어도 문제 없을거 같아요🤔
There was a problem hiding this comment.
죄송합니다. 구현하려는 방법은
if searchLocationsController.isActive {
return filteredLocations.locations.count
} else {
return allLocations.locations.count였는데 커밋이 이상하게 된 것 같습니다.
그리구 초기세팅은 allLocations.locations.count 로 정해도 되는데
서치바를 탭 했다가 다시 취소를 누르면 그 때는 allLocations.locations.count가 다시 등장해야 돼서
if-else로 나누어보았습니다.
| } | ||
|
|
||
| func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell { | ||
| let cell = collectionView.dequeueReusableCell(withReuseIdentifier: LocationCell.reuseIdentifier, for: indexPath) as! LocationCell |
There was a problem hiding this comment.
| let cell = collectionView.dequeueReusableCell(withReuseIdentifier: LocationCell.reuseIdentifier, for: indexPath) as! LocationCell | |
| guard let cell = collectionView.dequeueReusableCell(withReuseIdentifier: LocationCell.reuseIdentifier, for: indexPath) as? LocationCell else { | |
| return .init() | |
| } |
There was a problem hiding this comment.
나중의 실수를 고치기위해서는 이렇게 바꿔야 하는거군요! 감사합니다. 항상 !를 피할수 있는 방법을 먼저 생각하겠습니다.
| func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell { | ||
| let cell = collectionView.dequeueReusableCell(withReuseIdentifier: LocationCell.reuseIdentifier, for: indexPath) as! LocationCell | ||
| if searchLocationsController.isActive { | ||
| cell.locationNameLabel.text = filteredLocations.locations[indexPath.row].name |
There was a problem hiding this comment.
만약 filteredLocations.locations 데이터가 없는 상태에서 index 접근한다면 앱 크래시가 발생합니다. 데이터가 없는 경우, 예외적인 상황도 항상 고려해야 합니다!
There was a problem hiding this comment.
searchLocationsController.isActive가 되는 순간은 서치바를 클릭하는 순간이어서 reloadData()가 되면서 데이터가 없으면 filterdLocations가 없는데로 갱신이 되어서 Cell자체가 안보여서 앱 크래시가 뜰수 있는 상황은 없다고 생각했습니다.
혹시 제가 잘못 이해한 부분이 있을가요?
There was a problem hiding this comment.
제가 이 코드는 잘못 이해하고 있었네요. 말한 부분이 맞습니다. 죄송합니다!
| cell.locationNameLabel.text = filteredLocations.locations[indexPath.row].name | ||
| cell.locationCellImageView.image = UIImage(named: filteredLocations.locations[indexPath.row].imageName) | ||
| }else { | ||
| allLocations = dbManager.getLocations() |
There was a problem hiding this comment.
셀이 deque 될때마다 Realm 조회하고 조회된 객체가 생성하고 있을거 같아요! DB 자원을 읽고 객체를 생성하는 비용이 생각보다 작지 않습니다. 이 곳에서 데이터 조회해서 설정하는 이유가 무엇인가요?🤔
There was a problem hiding this comment.
아하! 생각해보니 맨 처음에 allLocations에 데이터가 저장되어있다면 그걸 이용하면 되는데 deque마다 불러주는군요!!
제가 생각이 짧았습니다. 큰 이유없이 제가 잘못 한거같습니다.
이 클래스에 init()을 해서 allLocations = dbManager.getLocations()를 최초에만 쓰고 그 이후에는 dbManager를 부르는 일을 없애도록 하겠습니다.
| import Foundation | ||
| import RealmSwift | ||
|
|
||
| private protocol DBOperations { |
There was a problem hiding this comment.
음.. 저는 이 DBOperations는 여기 DBManager에만 쓰기 때문에 private을 사용해서 다른 곳에서 못 부르는다고 생각했는데
굳이 private을 할 이유는 지금 검색해서보니 큰 의미가 없는 것 같네요.
항상 의미없는 옵션은 안 붙여보도록 하겠습니다.
There was a problem hiding this comment.
protocol 사용하는 목적을 생각한다면 private 접근 제어자가 어울리지 않다고 생각했어요. 그래도 필요하니깐 있는거겠죠ㅎㅎ
|
저는 DB를 이용해서 검색 뷰에 나타날 데이터를 표현했는데, 만약 네트워크로 구현했을 때에도 실제로 이런 정보들은 사용자 스마트폰에 저장이 되어서 재사용이 될지 궁금합니다.
|
|
감사합니다. 제가 지금 PR을 받고 수정을 해야하는데 다른 브랜치로 이미 진행을 하고 있어서 꼬였네요..ㅜ |
|
|
|
추가 코멘트 남겼습니다. 피드백은 생각해보고 지금 할 수 있는 것만 해보셔도 되요! 무리하지 않고 페이스 조절 잘하는게 더 중요합니다🙂 |
[FE] recoil: atoms, atom types 추가 (#45)
할인 정책이 여러개이므로 리스트에 저장해둠. 추후 Map이나 enum을 활용하는 등의 방법으로 리팩토링 생각해봐야할듯. 논리적으로는 list보다 set이 더 어울림. 가격정책이 세분화되면 인터페이스로 변경하는 식으로 추가해볼 수 있음. 인원 수에 따른 가격 계산 추가시켜야함. Dae-Hwa/airbnb/#45
평일에만 할인되도록 정책 추가 및 기본 할인 정책으로 설정. Dae-Hwa/airbnb/#45
구현 방법
사용한 프레임워크
구현한 패턴
메인 View 구성 요소
검색 View 구성 요소
데이터베이스
MVC 패턴
- 특히 Delegate 파일에서 updateSearchResults() [서치바에 텍스트 변경시 호출 함수] 에 따라 reloadData()를 하기 위한 작업으로 NotificationCenter를 적용해보았고 그로인해서 SearchLocation Controller에서 변경됨을 인지하고 reloadData()를 구현했습니다.
질문