[Team 6 iOS🥒 쏭 & 잭슨] MVVM 구조를 이용한 MainPage 구현

[JacksonPk]

작업목록

• : CollectionView와 CustomCell을 이용한 MainPage Layout 구성.
• : CocoaPods를 설치하고 Alamofire, Toaster Library 추가.
• : 목업API에서 네트워크를 이용해 정보 받아오기
• : MVVM구조를 적용해 Domain(Entity, UseCase), Prsentation(ViewModel, View) 구현.

학습 키워드

CollectionViewFlowLayout, CocoaPods, Alamofire, Toaster, MVVM, UICollectionReusableView

고민과 해결

• 팀원 두명 모두 MVVM에 도전하는 것이 처음이라 학습하는 마음으로 코드에 적용해보았습니다. 전체 구조를 처음부터 모두 구현하기 보단 현재 필요한 구조를 먼저 처리하는 것으로 방향을 잡았습니다.
• MVVM구조에서 switch-case를 사용하지 않기위해 OCP(Open-Closed Principle)를 적용해 보았습니다.

[ehgud0670]

Observable 도 직접 구현하시고, DTO라고 이름도 지으시고 고민의 흔적이 많이 보이네요! 💯 👍
피드백 드렸는데 확인 부탁드립니다. 고생하셨습니다~!!

[ehgud0670]

List 라는 표현은 프로그래머에게 특수한 의미입니다. 따라서 자료구조 중 하나인 List 가 아니라면 다른 이름으로 짓는 것이 어떨까요?(실제 컨테이너가 List인 경우라도 컨테이너 유형을 이름에 넣지 않는 편이 바람직합니다)

클린코드 2장 의미있는 이름 중 그릇된 정보를 피하라도 같은 내용이니 참고하셔도 됩니다!

[1song2]

오 아직 List 타입을 많이 사용해본 적이 없어서 그런 의미가 있었는지 몰랐네요. 알려주셔서 감사합니다!
클린코드도 읽어보겠습니다 :)

[ehgud0670]

접근이 너무 깊습니다. 디미터 법칙을 지키면 결합도를 효과적으로 낮출 수 있습니다. 뷰모델 객체를 만드셨으니 뷰모델 객체를 최대한 활용해보는건 어떨까요?

[1song2]

넵 viewModel 내부에서 item count를 반환하는 메소드를 만들어 보도록 하겠습니다!

[ehgud0670]

• DishesUseCase 라고만 해도 충분할 것 같아요(또 Fetch는 동사이기 때문에 처음 볼때 느낌이 메소드같기도 합니다).
• protocol을 좀 더 범용적으로 만드는 건 어떨까요? 프로토콜을 Dishes 관련되어서 제한적으로 설계하신 느낌이 듭니다. 좀 더 범용적으로 만들면 아래의 DefaultFetchDishesUseCase 이름도 자연스럽게 바뀔 것 같아요.
• execute의 (requestValue: FetchDishesUseCaseRequestValue)도 좀 더 범용적으로 만드는 것을 제안드립니다.
• execute의 requestValue 가 request 하는데 필요한 값만 가지고 있지 않네요! 이것도 적절하게 분리하면 좋을 것 같아요.

[1song2]

(1번 피드백) 오 UseCase는 각각의 시나리오를 나타내는 역할이라 (초기값 세팅, 아이템 추가 등...) 동사적 표현이 이름에 포함되지 않아야 할까 생각했는데 오히려 메소드 같은 느낌을 주는군요. MVVM이나 클린 아키텍처 예시 코드를 많이 보진 못했지만 이런 식으로 동사를 넣어 네이밍 하는 경우를 보긴 했는데, 그리 일반적인 컨벤션은 아닌걸까요?
(2번, 3번 피드백) 좀 더 범용적으로 수정해 다음번 PR에 반영하도록 하겠습니다.

(4번 피드백) execute의 requestValue 가 request 하는데 필요한 값만 가지고 있지 않다는 피드백이 혹시 FetchDishesUseCaseRequestValue 구조체의 속성인 categorizable의 전체를 사용하지 않고 그 내부 속성이 categorizable .name만 사용하고 있는 점을 지적하신 걸까요? 말씀하신 의미를 정확히 파악하지 못해 다시 한번 여쭙니다🙏

[ehgud0670]

(4번피드백) 네네 맞습니다. 서버쪽으로 request하는거라면 url이나 Request만 필요하지, category 자체가 필요할것 같진 않아서요!
그리고 category는 모델 객체의 느낌이 강한데 request 메소드에 어울리지 않다고 생각되어서요.

[1song2]

(4번 피드백) 현재 url 구조가 baseURL/{{category의 속성인 name}}이런 형태로 사용하고 있어 requestValue로 categorizable을 담아 보내고 있었었습니다. (e.g. baseURL/soup)
category 자체를 request하는 구조에서 category의 name만 요청하는 구조로 수정해보려 합니다!

[ehgud0670]

(1번 피드백) 아 넵, 이건 저도 이 레포를 보니 그대로 둬도 될 것 같다고 생각되네여.
저는 메소드는 동사로 시작하고 프로퍼티는 명사로 시작해야한다는 차원에서 말씀드린 건데 이 정도는 괜찮을 것 같아요, 명사로 프로퍼티 이름이 끝나기도 하고요.

[ehgud0670]

(2번, 3번 피드백) 네 최대한 범용적으로 만드는 거 추천드립니다. 그래야 왜 protocol을 사용하는지 와닿으실 것 같아요!

[1song2]

오 넵! 링크로 달아주신 레포를 많이 참고하여 MVVM을 구현했었습니다ㅎㅎ
UseCase는 최대한 범용적으로 사용해보고 어떤 점이 달라지는지 학습해보겠습니다!

[ehgud0670]

• 이 메서드는 인스턴스 메서드가 나을까요, 정적 메서드가 나을까요? 스위프트 내부 라이브러리의 정적 메서드가 어떤 역할의 메서드인지 확인하면서 고민하시면 좋을 것 같아요!
• 해당 메서드는 인스턴스 메서드로 만드시면 테스트하기에도 좋습니다. 의존성 주입, 가짜 객체 등 공부하시면 좋을 것 같아요 💪

[1song2]

static이 테스트가 어렵다는 것은 예전에 들었었는데 깜빡 생각을 못했네요!
말씀하신 키워드들도 공부해두도록 하겠습니다!

[ehgud0670]

• AF와의 의존성이 높네요. 다른 곳에서 하셨던 것처럼 생성자를 통한 의존성 주입 하는 것 어떨까요?
• 또 의존성 주입을 하실때 의존성 역전 원칙을 지키도록 protocol 타입을 만드시는 것도 추천드립니다.

[1song2]

Alamofire를 사용해 request 요청하는 부분을 따로 protocol로 구현해 NetworkManager가 채택할 수 있도록 하라는 말씀이실까요?
저희가 학습을 하면서 코드를 작성하다 보니 어느 부분이 말씀하신 생성자를 통한 의존성 주입을 구현한 부분인지 정확히 파악이 안되는 것 같습니다🥲 혹시 어떤 부분인지 조금 힌트를 주실 수 있을까요? 😭😭

[ehgud0670]

Alamofire를 사용해 request 요청하는 부분을 따로 protocol로 구현해 NetworkManager가 채택할 수 있도록 하라는 말씀이실까요?

=> 아 지금 보니, AF가 Networkmanager 자체의 역할을 해도 될 것 같습니다.

저희가 학습을 하면서 코드를 작성하다 보니 어느 부분이 말씀하신 생성자를 통한 의존성 주입을 구현한 부분인지 정확히 파악이 안되는 것 같습니다🥲 혹시 어떤 부분인지 조금 힌트를 주실 수 있을까요?

=> 위에대로 수정하면 각 Usecase 가 networkmanager를 생성자를 통한 의존성 주입 및 의존성 역전 원칙을 하면 되겠네요!
구글링해서 찾아보셔요 ㅎㅎㅎ

[1song2]

이 부분이 중요하기도 하고 저희가 학습이 많이 필요한 부분인 것 같아서 팀원과 좀더 꼼꼼히 공부해보고 그래도 이해가 어려우면 추가적으로 질문을 드릴 수도 있을 것 같아요. 말씀하신 것처럼 구글링이나 자료 찾아보면서 공부하고 좀더 개선해보겠습니다!

[ehgud0670]

뷰가 네트워크 단 역할까지 하면 안됩니다(뷰모델은 여러가지 변형케이스가 있기 때문에 허용됩니다).
이 부분도 분리해주세요. 뷰를 최대한 멍청하게 만들어야 합니다.

[1song2]

넵 기존에 가지고 있는 NetworkManager로 해당 코드를 옮기는 방향으로 수정해보겠습니다.

[ehgud0670]

섹션 타이틀을 모델이 가지고 있는게 좋을까요? 뷰모델이 가지고 있는게 좋을까요?
한글이 아닌 여러 다국어도 지원한다고 가정하고 생각해봅시다.
=> 이 부분은 저와 토론해봐요!! 저는 해당 sectionTitle값이 바로 섹션 헤더에 넣을수 있기 때문에 뷰모델이 가져도 된다고 생각했습니다.

[1song2]

현재 저희 코드가 섹션 타이틀을 모델이 가지고 있는 구조인가요, 아님 뷰모델이 가지고 있는 구조일까요? (멍청한 질문 죄송합니다😭)
저희끼리도 의견이 갈려서 한번 여쭤봐요.
@JacksonPk

• 모델 vs 뷰모델: 모델은 추상화된 데이터만 가지고 있어야 한다고 생각해서 섹션 타이틀을 뷰모델이 가지고 있는 구조 같습니다. 그래서 현재 구조를 유지하는 것이 좋다고 생각해요!
• 다국어 지원: 다국어를 추가한다면 categorizable 프로토콜에 다국어 변수를 추가하면 되지 않을까 생각합니다.

@1song2

• 모델 vs 뷰모델: 현재 뷰모델 폴더에 파일이 있긴 하지만 protocol과 struct만으로 이루어져 있기 때문에 모델이 섹션 타이틀을 가지고 있다고 생각합니다.
• 다국어 지원: 뷰모델에 필요한 언어별로 내부 속성이나 메소드를 추가해 해결할 수 있지 않을까 싶습니다.

[ehgud0670]

현재 저희 코드가 섹션 타이틀을 모델이 가지고 있는 구조인가요, 아님 뷰모델이 가지고 있는 구조일까요?
넵, 저는 뷰모델의 프로퍼티로 있는 모델(Category)이 섹션 타이틀을 갖고 있다고 생각했습니다.

• 모델 vs 뷰모델: 네, 저도 다시 보니 모델이 섹션 타이틀 값 그대로 가져도 된다고 생각합니다. 한글, 영어, 일본어 등등 모두 제공해야 한다고 해도 모델이 가져도 될 것 같아요.
• 다국어 지원: 저도 뷰모델이 각 지역에 따라 섹션 타이틀 값을 적절하게 줘야 한다고 생각해요. 모델이 여러 언어별 데이터를 가지고 있고 뷰모델이 취사선택하는 식으로요. 저는 뷰모델이 적절하게 모델값을 뷰에 넣어주는(제공하는) 역할로 이해했거든요.

=> 다 저의 생각이고 제안일 뿐이니, 의논하시고 진행하시면 될 것 같습니다!

[1song2]

다국어 지원 등 생각하지 못했던 부분도 공유해주셔서 감사합니다🙏

[ehgud0670]

toDomain()을 호출해서 DishList, Dish 객체로 바꾸셨지만, 그래도 제가 보기엔 networkmanager와 usecase의 역할의 차이가 없어 보입니다! 각 객체의 역할을 더 구분지어보는 건 어떨까요?

• 해당 블로그 글 보시는 것 제안 드립니다.
• JK의 포커앱 코드도 예시로 제안드립니다!

[1song2]

다시 코드를 보니 확실히 현재 FetchDishesUseCase와 NetworkManager가 큰 차이가 없는거 같네요.
DishCell에 있던 image를 받아오는 작업을 NetworkManager로 옮기고, 위 링크들도 참고하여 객체간 역할을 좀 더 구분해보도록 하겠습니다!

[ehgud0670]

• 보시면 아시겠지만 모두 세 번씩 메서드가 호출되거나 세 번씩 객체가 생성됩니다. 여기서 카테고리가 하나 더 추가되면 모두 한번씩 더 코드를 늘려줘야 합니다. switch 만 사용하지 않을뿐, 반복 호출이 되고 있는거죠.
• 세번씩 나열되는 이유가 endpoint 때문이라면 endpoint만 배열로 가지고 있는 건 어떨까요(아니면 endpoint 만 enum으로 만들어도 되고요)? 그 이후에 커버가 되지 않는 것은 다형적(프로토콜 or subclassing)으로 처리하는 겁니다(커버가 되지 않는 것은 예를 들어 로컬에서 생성하신 sectionTitle이 될 수 있을 것 같네요!).

[1song2]

넵 endPoint가 main, soup, side 세개라 세번씩 반복 호출을 해주고 있는데요, 먼저 ViewModel 생성하는 것은 endpoint 배열로 처리를 하고 그후 ViewModel을 이용해 load하거나 bind하는 작업은 ViewModel의 배열을 만들어서 처리해주면 되는걸까 생각이 드네요. 좀더 고민하면서 더 좋은 구조로 고칠 수 있을지 시도해보겠습니다.

[ehgud0670]

뷰모델이 단순히 모델 객체를 하위 객체로 가져도 되지 않을까요?

[1song2]

뷰모델이 단순히 모델 객체를 하위 객체로 가져간다는 말씀이 Dish entity와 DishesListItemViewModel 뷰모델이 큰 차이가 없으니 Dish를 내부 속성으로 하나 가져가면 된다는 의미실까요?

[ehgud0670]

네네 맞아요, 제가 이해한 MVVM 패턴은 뷰모델이 모델을 가지고 있고, 뷰모델이 뷰와 소통하는식으로 이해했거든요. 뷰모델이 동작(메소드)로 뷰에 넣을 값을 모델값을 적절하게 이용해서 넣어주는 거죠.
지금 DishesListItemViewModel 은 모델객체인 Dish와 아직 차이가 없지만 차차 동작이 생길 것 같아요.

[1song2]

아 넵 무슨 말씀이신지 이해했습니다. 그렇게 수정하게 되면 viewModel.name에서 viewModel.dish.name과 같은 구조로 수정을 해야할 것 같은데 디미터 법칙과는 약간 멀어지는게 아닌가 싶지만 그리 크게 해치는 것도 아닌가 싶긴 하네요.

[ehgud0670]

아 디미터 법칙도 고려해서 구현하신거군요. 그러면 view에 fill 메소드에서 dish를 지역변수(let dish = viewModel.dish로 만들어서 하면 해결될 것 같군요
중요한 건 뷰 - 모델 - 뷰모델를 구분지어서 프로퍼티도 정하시는것 같습니다. 지금은 뷰모델이 모델의 역할도 하고 있는 것 같아서요.

[1song2]

꼼꼼한 리뷰 정말 감사드립니다! 아직 확인을 못한 부분은 점심 먹고 와서 후딱 또 코멘트 달도록 하겠습니다! 맛점하세요🍽

[1song2]

제이슨, 저희가 질문이 정말 많았는데 친절하고 상세하게 답변해주셔서 정말정말 감사드려요🥲🙏
말씀해주신 부분을 다 수정하고 다음 기능 구현으로 넘어가면 좋겠지만 이번 주 금요일이 데모일이라 일정이 좀 빠듯할 것 같습니다.

기능 구현과 동시에 피드백 주신 부분 수정하고 고민이 많이 필요한 부분은 데모 후에도 생각을 해보려 하는데 괜찮으실까요?

다시 한번 리뷰 감사드립니다!!!

[ehgud0670]

넵넵 생각이 필요한 부분은 데모이후에 하셔도 됩니다!
지금 수정할 수 있는 부분 수정하시면 제가 확인하고 머지 할게요!

[ehgud0670]

하지만 데모전에 한번 더 pr 날리시는거 추천드립니다. 이 두 가지중에 하나를 택해서요.

1. 네트워크 단 테스트 코드를 작성한다. 가짜 객체(Test Double)를 이용한다.
2. 한 섹션의 데이터를 받아오면 해당 섹션만 먼저 뷰 업데이트한다. <- 동시성 이슈가 있고 다른 플젝에서는 시도 할 수 없습니다. 이 프로젝트의 메인 디쉬에요 ㅎㅎ

데모를 앞두고 최대한 많이 구현해서 보여주고 싶어하는 마음 이해합니다 ㅠㅠ 하지만 위의 두가지를 진행하시면 단순히 뷰만 구현하는 것 보다는 더 좋은 경험일 것 같아서요. 데모 끝나고도 pr은 받을테니깐 편하신대로 진행하시면 될 것 같아요!

[1song2]

하지만 데모전에 한번 더 pr 날리시는거 추천드립니다. 이 두 가지중에 하나를 택해서요.

1. 네트워크 단 테스트 코드를 작성한다. 가짜 객체(Test Double)를 이용한다.
2. 한 섹션의 데이터를 받아오면 해당 섹션만 먼저 뷰 업데이트한다. <- 동시성 이슈가 있고 다른 플젝에서는 시도 할 수 없습니다. 이 프로젝트의 메인 디쉬에요 ㅎㅎ

데모를 앞두고 최대한 많이 구현해서 보여주고 싶어하는 마음 이해합니다 ㅠㅠ 하지만 위의 두가지를 진행하시면 단순히 뷰만 구현하는 것 보다는 더 좋은 경험일 것 같아서요. 데모 끝나고도 pr은 받을테니깐 편하신대로 진행하시면 될 것 같아요!

넵 제이슨! 일단 피드백 주신 내용 중 오늘 안에 수정할 수 있는건 오늘 저녁 or 밤 다시 push한 후 코멘트 달겠습니다.

말씀해주신 두가지 중에 사실... 두번째는 구현이 되어 있다고 생각했는데 아닌가 봅니다😭😭
시뮬레이터로 구현해보았을 때 순차적으로 반찬 메뉴가 뜨는데 더 개선해야 할 부분이 있을까요?

기능 구현이 많이 되지 않아 불안한 마음이 있었는데 제이슨 말씀 듣고 진짜 중요한 게 뭔지 다시 깨닫게 되는거 같아요. 정말 정말 감사합니다🙏

[JacksonPk]

제이슨. 오늘 피드백 주신 내용 중 수정 가능한 부분 고쳤습니다. 고친 내용과 앞으로 고쳐야 할 내용은 아래 링크에 기록해 두었습니다.
rla36#23
오늘 하루 정말 감사드립니다. 맛저하시길!!😋

[ehgud0670]

넵 @1song2, @JacksonPk 님, 근무중이라 퇴근하고 지금 봅니다!

• 제가 말씀드린 두번째는 reloadData가 아닌 reloadSections를 사용해야 해당 섹션만 업데이트 했다고 할수있습니다..! 저 포함 작년 멤버들도 과정 밟을때 JK의 피드백을 듣고서야 알았습니다. 이 부분 데모 전이나 후에 꼭 도전해보셨으면 좋겠습니다!
• 네네 ㅎㅎ 저도 마스터즈 코스 수강중일때 많은 걸 보여줘야 한다는 압박감이 있어서 스트레스가 많았는데 지나고보니 그럴 필요는 없더라구요.
  한 프로젝트 당 자기가 얻어갈수 있는 것만 잘 얻어가시면 충분하다고 봅니다, 따라서 매번 똑같은 걸 하는것보단 여러가지를 하는걸 추천드려요.

[ehgud0670]

@JacksonPk 넵, 확인하고 머지하도록 하겠습니다.
변경사항들 정리해주셔서 감사합니다 👍

[ehgud0670]

고생하셨습니다! 제 피드백 하나만 반영하시면 머지할게요.
수고하셨습니다~!! 👍💯

[ehgud0670]

잘 구현하셨네요! 구현하니깐 나머지 부분은 속이 시원하시죠? ㅎㅎ

[ehgud0670]

오 넵 잘 하셨어요! 박싱된 타입(Box<T>)에는 ?를 붙이지 않는것을 잊지 않으시길 바랍니다 ㅎㅎㅎ

[1song2]

넵 명심해두겠습니다!

[JacksonPk]

넵 박싱된 타입은 non-optional !!

[ehgud0670]

넵, 뷰에서 뷰모델을 프로퍼티를 두지 않는 것 잘하셨습니다!!
하지만 이미지를 서버로부터 가져오는 역할도 뷰 안에서 이뤄지면 안됩니다.
뷰는 오직 렌더링하는 역할만 해야 하기 때문입니다.
다음번에는 networkManager도 빼셔야 돼요~~

[1song2]

아 피드백 주신 부분을 완전히 이해하지 못했었네요! 다음번 PR 드리기 전에 수정해놓도록 하겠습니다!

[ehgud0670]

넵~~

[ehgud0670]

음 제가 설명을 잘 못드린것 같네요 ㅜㅜ
제가 말씀 드린 자료구조는 Set, List, Array, Dictionary 등등이 모두 포함됩니다.

• 해당 자료구조를 쓰지않으면 이름에 위의 자료구조들이 포함되선 안됩니다.
• 또 해당 자료구조가 쓰인다고 해서 변수명을 dishArrs 보단 dishes가 적절합니다. 왜냐하면 언제 또 자료구조 타입이 바뀔지 모르니깐요.

=> 이 부분만 수정하시면 머지할게요. 단순히 Dishes도 괜찮을 것 같은데요? 중복되더라도요.

[1song2]

아하 부연설명 주신 덕분에 완전히 이해했습니다!
팀원 @JacksonPk 도 Dishes로 수정하는 것 어떠냐고 의견 주셨었는데, 내부 프로퍼티로 dishes가 있어 너무 중복되는 것 같아 망설여지더라구요😭 하지만 설명해주신 내용 들어보니 DishList나 DishSet 보다는 Dishes가 좋은 것 같습니다!
(일단은 Dishes로 수정한 후 내부 프로퍼티를 다른 이름으로 바꿀 수 있을까도 고민해보겠습니다!)

[ehgud0670]

넵넵, 완전히 이해하셨다니 다행이네요 ㅎㅎ
근데 해당 경우는 내부 프로퍼티와 중복되어도 저는 괜찮을 것 같습니다.
왜냐하면 포커앱의 경우에도 Players라는 객체가 players 배열을 프로퍼티를 그대로 갖고 동작(메소드)을 추상화하기 때문이죠.
다만 클래스(또는 구조체)로 추상화했음에도 동작(메소드)가 없는 것들은 추상화가 필요없는 것들이니 삭제하셔도 될 것 같습니다!

[1song2]

제이슨, DishSet 대신 Dishes로 수정해 push 해두었습니다!
오늘 하루종일 너무 고생 많으셨어요. 피드백 주신 내용들 통해 정말 많이 배울 수 있었습니다!
감사드려요. 그럼 좋은밤 되세요🌝

[ehgud0670]

오 넵 피드백 반영 감사합니다! 😀
네 ㅎㅎ, 저도 하면서 많이 배웠습니다! 리뷰하면서 리뷰어도 많이 성장하는것 같아요 ㅎㅎ
그럼 머지할게요, 좋은 밤 되세요~~

[JacksonPk]

감사합니다, 제이슨 그리고 쏭! 모두에게 성장하는 시간이 되어서 좋은 날입니다. 좋은 밤 되세요!!