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

[team-05][만사,V] 메인화면 구성 & MockAPI 연결 #6

Merged
merged 34 commits into from
Apr 27, 2021

Conversation

zzisun
Copy link

@zzisun zzisun commented Apr 21, 2021

프로그래밍 요구사항

  • UICollectionViewDiffableDataSource를 사용하여 UI구성
    서버의 JSON 데이터를 받아오는 모델 객체를 만든다. Combine 활용
  • HTTP 프로토콜 GET 요청으로 서버의 API에서 메인반찬 JSON 데이터를 받는다.
  • HTTP 프로토콜 GET 요청으로 서버의 API에서 국.찌게 JSON 데이터를 받는다.
  • HTTP 프로토콜 GET 요청으로 서버의 API에서 밑반찬 JSON 데이터를 받는다.
  • 응답으로 받은 JSON 데이터를 사용하여 테이블뷰에 보여준다.
  • 모델 객체는 응답이 도착하면 섹션별로 데이터를 갱신하고 각각 Notification을 보내서 테이블뷰의 해당 섹션만 업데이트한다.
  • 상세 화면에 대한 요청은 상품을 골라서 화면이 표시하기 직전에 요청해야 한다.

@Sonjh1306 Sonjh1306 added the review-iOS iOS 리뷰 label Apr 21, 2021
@ehgud0670 ehgud0670 self-requested a review April 21, 2021 22:18
@zzisun zzisun changed the title [iOS][만사&V] 메인화면 구성 & MockAPI 연결 [team-07][만사,V] 메인화면 구성 & MockAPI 연결 Apr 22, 2021
import Combine

class SidedishViewController: UIViewController {
typealias FetchData = () -> ()

Choose a reason for hiding this comment

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

이 이름도 Model 이름 처럼 보이지 클로저 처럼 보이진 않습니다. 다름 이름으로 변경 부탁드려요
ex) XXXBlock XXXHandler

@zzisun zzisun changed the title [team-07][만사,V] 메인화면 구성 & MockAPI 연결 [team-05][만사,V] 메인화면 구성 & MockAPI 연결 Apr 22, 2021
@ehgud0670
Copy link

ehgud0670 commented Apr 22, 2021

  • 정정합니다. UICollectionViewDiffableDataSource 를 사용하려면 Section을 enum화 시키는 게 좋겠네요 ㅠㅠ ㅋㅋ
    • 다만 Section의 rawValue로 indexpath.section 값을 바로 넣는 건 좋지 않은 것 같습니다. 다른 방법을 찾아보세요.
    • 그리고 enum을 사용하시더라도 switch 사용은 최소화 하시는것 추천해요.
  • 동시성 문제는 제가 UICollectionViewDiffableDataSource를 안써봐서 모르겠는데 일어나지 않을 수도 있을 것 같습니다. 하지만 snapshot 부분은 pure하게 만드는것이 좋을 것 같아요.

Comment on lines 14 to 17
enum Section: Int, CaseIterable {
case main
case soup
case side

Choose a reason for hiding this comment

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

Section, DataItem, SupplementaryElementKind 등도 SideDishVC와 분리되어도 될 것 같습니다.
enum 선언 먼저보이니 VC 코드 파악이 조금 힘드네요

Choose a reason for hiding this comment

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

네네 ! 분리시키기는 했는데 분리를SidedishVC파일 안에 시켰는데, 아예 다른 파일로 분리시키기에는 SidedishVC와 연관성이 좀 있다고 판단을 했는데 리뷰어님 의견도 궁금합니다.ㅎㅎ

Choose a reason for hiding this comment

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

  • 아 넵 저는 Section, DataItem, SupplementaryElementKind이 해당 VC에서 사용하는 collectionView(UICollectionViewDiffableDataSource 포함)와 관련된 것들이라 해당 VC말고 다른 VC에도 그대로 사용하는 경우에 또 필요할 것 같다는 생각이 들어서 말씀드렸습니다!
  • 그리고 해당 VC에서 collectionView가 아닌 다른 뷰들도 포함되는 경우에는 VC의 코드가 늘어나게되는데 저의 경우에는 ViewController 코드 처음 부분에 프로퍼티들이 위치 하지 않으면 코드 파악하는데 조금 힘들더라고요. 그래서 말씀드렸어요 ㅎㅎ
  • //MARK:-를 표시해서 코드를 구분하는것도 추천드립니다.

Choose a reason for hiding this comment

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

나중에 여러 VC에서 해당 collectionView와 저 enum들(Section, DataItem, SupplementaryElementKind)을 그대로 사용한다고 생각하면, 아예 파일을 나눌 수도 있을것 같아요. 👍

Comment on lines 47 to 50
fetchMainData()
fetchSoupData()
fetchSideData()
}

Choose a reason for hiding this comment

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

이 부분도 함수 나열해서 호출하는 것 말고 다른 방법으로 개선할 수 있을 것 같습니다.

Choose a reason for hiding this comment

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

나름대로 수정하긴 했는데 viewDidLoad에 나열해서 사용하는 것과 private func로 묶어서 viewDidLoad에서 호출하는 방식으로의 개선이 올바른 개선일까요? 어느 수준까지 개선을 해야할지 애매하네요.ㅠㅜ

Choose a reason for hiding this comment

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

넵넵 힌트는 왜 Section을 기준으로 enum을 사용하게 되었는가? 입니다

Choose a reason for hiding this comment

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

  • 사실 섹션별로 뷰는 변한게 없고 요청하는 url 값만 다릅니다. 만약에 요청하는 카테고리 케이스가 늘어나게 되면 fetchXXXData 가 또 생길거에요. enum을 사용하신다면 무엇을 enum으로 만들고 왜 만드는지 한번 생각해보세여.
  • 그래서 저의 경우에는 요청하는 url 배열(부끄럽지만 예시를 위해 링크 드립니다ㅜㅜ ㅋㅋ)을 데이터로 먼저 내려달라고 백엔드께 부탁드렸었습니다. 하지만 이럴 필요까지는 없고 요점은 enum을 어떻게 잘 쓸 것인가 같아요.

self.fetchData(fetchData: self.sidedishViewModel.fetchSideData, path: path)
}

private func fetchData(fetchData handler: FetchDataHandler, path: String) {

Choose a reason for hiding this comment

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

Suggested change
private func fetchData(fetchData handler: FetchDataHandler, path: String) {
private func fetchData(handler: FetchDataHandler, path: String) {

=> 메소드 이름과 중복되네요, 해당 부분도 제거하면 더 좋을 것 같습니다

@Malloc72P Malloc72P merged commit b118913 into codesquad-members-2021:team-5 Apr 27, 2021
ksundong pushed a commit that referenced this pull request Apr 28, 2021
- 응답 바디 JSON 구성을 위한 객체 추가

issue: #6
ksundong pushed a commit that referenced this pull request Apr 28, 2021
- ItemDetailController를 추가한다

issue : #6
ksundong pushed a commit that referenced this pull request Apr 28, 2021
- 섹션 컨트롤러를 추가하였습니다.

issue: #6
ksundong pushed a commit that referenced this pull request Apr 28, 2021
- JSON 표기할 때 순서를 정하기 위해서 Comparable을 구현

issue: #6
ksundong pushed a commit that referenced this pull request Apr 28, 2021
- SectionController에서 detail을 제외하고 매핑하도록 regex 추가

issue: #6
ksundong pushed a commit that referenced this pull request Apr 28, 2021
[BE] Controller 구현

close #6
ksundong pushed a commit that referenced this pull request Apr 28, 2021
- 파비콘 요청 url이 오는 에러가 발생해서 @RequestMapping url 수정

issue: #6
crongro pushed a commit that referenced this pull request Apr 30, 2021
* chore: Initialize structure

폴더 구조 짜봤습니다 ㅎㅎ

* docs: Add database schema mwb,png file

오늘 관련 스키마 설계하여 mwb와 png파일 추가하였습니다.

* [#1] chore: Test commit

* Revert "[#1] chore: Test commit"

This reverts commit 9546450.

* chore folder 구성

* [#6] 캐러셀 슬라이드 구현 완료

* feat: header 정적 ui 작성

* feat: header 하위 레이어 이벤트 구현

* chore: 불필요한 파일, 주석 제거

* feat: tab ui

* [#6]feat: Carousel Slide 완성

* [#11] feat: PopUp Modal frame 완성

* chore

* feat: Carousel Slide Content

* feat: Fetch tab ui data

* feat: Modal

* [#11] feat:PopUp Modal

* fix: ui별 각각 스타일 적용

* feat: style, model tab ui 적용

* refactor: Carousel

* feat: 모달 세부이미지 클릭 이벤트 적용

* feat: 캐로셀 리펙토링+모달 추가기능 합침

* feat: 주문하기

* feat: POST order && PopUp Carousel

* feat: PopUp Carousel

* refactor: PopUpModal

* refactor: PopUpModal 클릭시 best Modal과 함께 뜨는 오류 수정

* chore: add gitignore

* Update .gitignore

* chore

* chore

Co-authored-by: 정이삭 <isaac56@naver.com>
Co-authored-by: reminderclock <reminderclock@naver.com>
Co-authored-by: reminderclock <71510362+reminderclock@users.noreply.github.com>
HoonHaChoi pushed a commit that referenced this pull request Apr 30, 2021
ksundong pushed a commit that referenced this pull request May 1, 2021
- 스프링 부트 프로젝트 기본 셋업
- 프로젝트 상위 폴더에 .gitignore 파일 추가
ksundong pushed a commit that referenced this pull request May 1, 2021
- application.properties에 datasource 설정 추가
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-iOS iOS 리뷰
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants