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

[iOS] 메인화면, 필터화면 뷰 구현 #57

Merged
merged 153 commits into from
May 27, 2020
Merged

[iOS] 메인화면, 필터화면 뷰 구현 #57

merged 153 commits into from
May 27, 2020

Conversation

seizze
Copy link
Contributor

@seizze seizze commented May 26, 2020

구현한 내용

  • 메인 화면의 검색창, 필터 버튼을 구현하였습니다.
  • 날짜, 인원, 가격 버튼을 터치했을 때 나오는 Filter View Controller를 구현하였습니다.

고민한 내용

  • 페어 간 코딩 컨벤션을 맞추가 위해 노력하였습니다.
  • 비슷한 종류의 버튼끼리(날짜, 인원, 가격 버튼) 상속 관계를 구성하였고, 각 버튼이 필터 타입을 갖고 있도록 만들었습니다.
  • 버튼을 눌렀을 때 Filter VC를 띄운 뒤 버튼의 타입을 전달하여 필터 타이틀을 설정하도록 구현하였습니다.
  • 스토리보드에서 특정 VC를 로드하는 유틸을 구현하였습니다.
  • 특정 VC를 띄울 때 필요한 데이터를 전달하면서 생성하기 위해 Filter View Controller에 instantiate 메서드를 구현하였습니다.
  • ImagePagingView: 이미지를 페이징 방식으로 스크롤할수 있는 커스텀 뷰입니다.
    • ImagePagingView를 처음에 인터페이스 빌더에서 구현하려했으나 scrollView에 자식뷰로 empty한 stackView를 가지고 있는 경우 scrollView의 FrameLayoutGuide의 width를 줄 수가 없어서 코드로 구현하였습니다.
    • ImagePagingView에서 처음에 image_url의 개수대로 stackView나 pageControl을 설정하기 위해 configure(count: Int)를 구현하였고, 이후 사진을 한개씩 다운로드하는 비동기적인 상황에서 사진을 순서대로 받기 위해 insert(at index: Int, image: UIImage)을 구현하였습니다. 그런데 이 부분은 이후 구현할 이미지를 다운받을 유즈케이스 객체나 뷰모델과의 관계를 따져보고 바꾸어야 한다면 바꿀 예정입니다.

구현 결과 화면

  • ImagePagingView (기능을 체크하기 위해 임의로 추가하고 삭제하였습니다.)

봄날은_간다

  • 나머지 화면들

result1

seizze and others added 30 commits May 19, 2020 14:16
[iOS] 탭바 컨트롤러 스토리보드로 변경
	* frame, coder 생성자 붙임
	* final 로 선언함 ( 상속 x )
	이유: 문자 표현에 대한 값은 로컬라이즈를 고려해 뷰모델에 위치하는게 낫다.
	      그리고 벳지 레이블은 다른 화면에서도 사용하기때문에 벳지 레이블만의 뷰모델이 있는 것은 합당하다고 생각해서 생성했다.
@seizze seizze changed the title [iOS] 메인화면 뷰 구현 [iOS] 메인화면, 필터화면 뷰 구현 May 26, 2020
@seizze seizze changed the title [iOS] 메인화면, 필터화면 뷰 구현 [iOS] 메인화면, 필터화면 뷰 구현(작성중) May 26, 2020
@seizze seizze requested a review from godrm May 26, 2020 09:45
@seizze seizze changed the title [iOS] 메인화면, 필터화면 뷰 구현(작성중) [iOS] 메인화면, 필터화면 뷰 구현 May 26, 2020
@seizze seizze merged commit c079e91 into dev May 27, 2020
@godrm
Copy link
Contributor

godrm commented May 27, 2020

이번 PR은 범위가 조금 큰 것 같네요. 커밋도 많고 파일도 많네요 헥헥
우선 ImagePagingView 를 만든 것과 메인 하면을 만든 것도 서로 큰 관련이 없어 보이니까 분리하는 게 좋았을 것 같습니다.
PR 단위도 한 번 고민해보세요.


import UIKit

enum Storyboard: String {
Copy link
Contributor

Choose a reason for hiding this comment

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

이런 일반적인 이름(애플이 쓸 것만 같은)을 타입으로 선언할 때는 겹칠수도 있어서 조심해야 합니다.
우리는 좀 더 구체적인 이름을 사용하게 좋습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 좀 더 구체적인 이름인 StoryboardRouter로 변경하였습니다! 860a948

let `super` = super.intrinsicContentSize
return CGSize(
width: `super`.width + horizontalInset * 2,
height: `super`.height + verticalInset * 2
Copy link
Contributor

Choose a reason for hiding this comment

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

오 이런 방식을 쓰다니 어색하지만 납득은 되는 표현이네요.

Copy link
Contributor

@ehgud0670 ehgud0670 May 28, 2020

Choose a reason for hiding this comment

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

아하 넵 super 라는 상수를 지우고 바로 super.intrinsicContentSize를 할당하도록 하였습니다.

79e5d4d


final class GuestsButton: FilterButton {
override func invokeAction(sender: FilterButton) {
action?()
Copy link
Contributor

Choose a reason for hiding this comment

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

override 메소드인데 super.xxx 를 호출하지 않아도 되는걸까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

원래는 상위 클래스인 FilterButton의 invokeAction은 비어있고, 하위 클래스에서 override하여 구체적인 동작을 하도록 하려는 의도였었습니다. 비어 있기 때문에 super.xxx를 호출하지 않았었습니다.

하지만 FilterButton의 invokeAction을 구현하여 이 클래스를 상속한 모든 버튼에서 공통된 동작을 하도록 만들 수도 있기 때문에 super.invokeAction을 먼저 호출하도록 해 놓는 것이 더 낫겠다는 생각이 들어서 수정하였습니다! 933714c

struct Math {
static func modular(of dividend: CGFloat, divideBy divisor: CGFloat) -> CGFloat? {
guard divisor != 0 else { return nil }
return dividend.truncatingRemainder(dividingBy: divisor)
Copy link
Contributor

Choose a reason for hiding this comment

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

한 가지 팁을 주자면 CGFloat로 연산하는 게 항상 좋은 선택은 아닙니다.
픽셀 정보가 소숫점으로 처리가 가능하지만, 실제로 iOS 좌표값은 정수로만 떨어집니다.
그래서 정수로 계산하고 값을 넣을 때만 CGFloat로 변환하는 게 좋은 경우가 많습니다. 참고하세요.

Copy link
Contributor

@ehgud0670 ehgud0670 May 28, 2020

Choose a reason for hiding this comment

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

네, 메소드 modular와 `quotient의 파라미터 타입을 모두 CGFloat이 아닌 Int로 바꾸었습니다.
좋은 꿀팁 감사합니다 ㅎㅎ

28ad238

self.measure {
// Put the code you want to measure the time of here.
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

다음에는 테스트 케이스로 테스트할 수 있는 모듈을 단위 테스트를 추가해보면 어떨까요?
이제 단위 테스트에 대한 실전과 이론을 배우고 있으니까요.
테스트 가능한 구조로 만드는 건 또 다른 느낌이 됩니다.
화면 없이 동작하는 모듈이 많으면 더 테스트하기 좋을 겁니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

넵, 감사합니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵!!

@seizze
Copy link
Contributor Author

seizze commented May 28, 2020

피드백 반영 완료했습니다. 🔥

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

Successfully merging this pull request may close these issues.

None yet

3 participants