[team-09][Lollo, ZG] 데이터 받아서 persistence(CoreData)에 저장하고 불러오기 #48

Settpark · 2021-04-28T07:27:28Z

작업내용

기존에 네트워크 바로 UseCase로 가던 흐름 중간에 Repository를 추가하였습니다.
mock API에서 백엔드에서 배포한 API로 변경하였습니다.
원래 기획한 내용은 업데이트가 필요할 경우 네트워크에 요청을 하고 아닐 경우, CoreData에서 호출하는데, 구분하는 로직을 구현하지 못하여 계속 네트워크 부터 흐름이 시작되고 따라서 TurnonAppUsecase.swift 26번째 self.repository.deleteAllINCoreData()로 계속 내부 CoreData를 삭제한 후 재 저장하고 저장되어 있는 데이터를 불러오도록 하였습니다.

고민 거리

repository에서 combine을 사용하고 있으나 마땅한 방법이 떠오르지 않아 마지막 처리를 escaping closure를 통해서 처리했습니다. combine에 대한 이해도가 조금 낮아서 이렇게 작성 하였는데 저번에 리뷰 해주신 alertMessage의 내용과 겹치는 것 같아 작성한 내용의 방향성이 맞는 지 여쭤보고 싶습니다.

프로토콜로 변경함에 따라 내부 속성을 가져오는 임시함수 선언 및 사용

- 배지를 stack view에 넣어 유동적으로 표시할 수 있도록 수정

- Footer 추가 - Image View 사이즈 조정

…into fix/ios/mainUI

CoreData 추가, Repository 클래스 추상화

굳 :)

정말 굿입니다! :)

고생했어요 :)

pay-napster-x

Repository가 생겼군요 !🥰 잘하셨습니다.
다만 몇가지 피드백을 드리니 확인해보세요.
수고하셨습니다!

구분하는 로직을 구현하지 못하여 -> 구분하는 로직은 아직 구현하지 못하신건가요?

pay-napster-x · 2021-04-29T01:10:58Z

ios/sidedish/sidedish/Data/CoreDataClass/SaveSideDish+CoreDataProperties.swift

+        static let badges = "badges"
+    }
+
+    func getTitle() -> String {


public속성이면 get 메소드들은 불필요할 것 같습니다. get은 Java 컨벤션이니 스위프트 처럼 작성해 보세요. :)
Public으로 var 로 선언한 이유가 있을까요?

pay-napster-x · 2021-04-29T01:12:33Z

ios/sidedish/sidedish/Data/CoreDataClass/SaveSideDishes+CoreDataProperties.swift

+    @NSManaged public var id: Int16
+    @NSManaged public var sideDish: [SaveSideDish]?
+
+    enum Properties {


enum에 static을 사용한 이유는 무엇인가요? 단순히 상수값을 나타낸다면 struct에 static을 사용해도되지 않을까요?
enum을 사용한 이유는 무엇인가요?
case에 description 타입을 이용하면 enum을 사용해도 될 것 같은데,
마치 C언어처럼 보입니다.

pay-napster-x · 2021-04-29T01:15:48Z

ios/sidedish/sidedish/Data/CoreDataClass/SaveSideDish+CoreDataProperties.swift

+    }
+}
+
+extension SaveSideDish : Identifiable {


extension으로 따로 선언하신 이유가 있을까요?
본체에 id때문에 Identifiable을 써서 구분하신것 같은데,
이렇게 따로 빼놓으신 이유가 궁금합니다.

pay-napster-x · 2021-04-29T01:19:18Z

ios/sidedish/sidedish/Data/CoreDataStorage.swift

+import Foundation
+import CoreData
+
+final class CoreDataStorage {


싱글톤을 이용한 이유가 궁금합니다. :)
싱글톤을 쓰면 객체들간의 결합도를 높이고, 응집도를 낮추는데, 꼭 싱글톤이 필요한 이유가 있어야할 것 같습니다.
마찬가지로, 싱글톤이 아니라면 persistentContainer 속성을 만들때 name에 SideDish의 값을 내부에서 생성하지말고
외부에서 생성하는건 어떨까요? 단순히 SideDish만을 불러오기위해 이렇게 바로 할 순 있지만,
최소한의 따로 SideDish라는 상수로 빼서 사용해야 나중에 수정에 좀더 유리한 구조가 됩니다.
😃

pay-napster-x · 2021-04-29T01:20:20Z

ios/sidedish/sidedish/Data/CoreDataStorage.swift

+                try context.save()
+            } catch {
+                let nserror = error as NSError
+                fatalError("Unresolved error \(nserror), \(nserror.userInfo)")


항상 저장이 성공한다는 보장을 할 수 없습니다.
에러가 나면 어떻하죠? 🥶

pay-napster-x · 2021-04-29T01:58:06Z

ios/sidedish/sidedish/Presentation/MainMenuList/View/MenuCell.swift

        }
+
+        self.eventBadge.isHidden = badge.contains(BadgeType.event.description) ? false : true


badge.contains(BadgeType.event.description) ? false : true
이 로직은 정말 View에서 해야하는 걸까요?
View는 그려주기만 하는 역할을 해야합니다.
결국 View의 BadgeType은 Entity의 값을 알아야 그릴 수 있게 되어버렸습니다.
받아서 그리기만 하면 될 것 같습니다.

pay-napster-x · 2021-04-29T04:36:28Z

ios/sidedish/sidedish/Data/DishRepository.swift

+            if let sideDishEntity = self.sideDishEntity {
+
+                var sideDishes = [NSManagedObject]()
+                dishes.forEach {


class MyClass { var id: Int init(id: Int) { self.id = id } } func test() { let testArray = ["1", "2"] let my = testArray.flatMap { MyClass(id: Int($0) ?? 0) } }

flatMap으로 초기화는건 어떨까요?

pay-napster-x · 2021-04-29T04:37:13Z

ios/sidedish/sidedish/Data/DishRepository.swift

+
+    private func updateCategory(of endPoint: String, with sideDishes: [NSManagedObject]) {
+        let fetchRequest = findCategoryForEndPoint(endPoint)
+        let objectToUpdate = try! self.context.fetch(fetchRequest).first!


강제 언랩핑을 최대한 줄여보세요 😥

pay-napster-x · 2021-04-29T04:44:51Z

ios/sidedish/sidedish/Presentation/MainMenuList/ViewModel/MenuCellViewModel.swift

-    @Published var dishesCategory: [SideDishesCategory]!
-    @Published var dishes: [[SideDish]]!
+    @Published var dishesCategory: [SideDishesCategoryManageable]!
+    @Published var dishes: [[SideDishManageable]]!
    @Published var errorMessage: String!


만약에 초기화가 안된 errorMessage 강제언랩핑에 접근해버리면 어떻게 될까요?

pay-napster-x · 2021-04-29T04:55:52Z

ios/sidedish/sidedish/Presentation/MainMenuList/ViewModel/MenuCellViewModel.swift

        }
    }

    private func loadSideDishes(count: Int) {
        self.dishes = Array(repeating: [], count: count)
        for i in 0..<count {
-            turnonAppUsecase.manufactureForMainViewSideDishes(endPoint: EndPoint.sideDishes[i])
-                .sink { (result) in
+            turnonAppUsecase.manufactureForMainViewSideDishes(endPoint: EndPoint.sideDishes[i]) { (publisher) in


각 섹션별로 서버를 요청하는거군요?
0~count까지 요청을하는데 만약에 순서대로 응답이 안들어와도
괜찮은건가요? 🤔 어떻게 생각하시나요

맞습니다! 섹션 별로 요청합니다
순서대로 들어오지 않으면 당장 보는 화면이 로드되지 않을 수도 있어서 좋지 않을 것 같습니다..
사실 lazy scrolling 방식으로 구현을 해보려고 섹션 별로 서버 요청이 가능하도록 구현했는데,
그 부분을 구현하지 못해서 더 어색해진 것 같습니다🥲

순서대로 API가 항상 오지는 않을 꺼에요. Combine을 사용하면 순서를 보장할 수 있을 것 같은데 어떻게 하면 될까요?

pay-napster-x · 2021-04-29T04:59:54Z

Q

repository에서 combine을 사용하고 있으나 마땅한 방법이 떠오르지 않아 마지막 처리를 escaping closure를 통해서 처리했습니다. combine에 대한 이해도가 조금 낮아서 이렇게 작성 하였는데 저번에 리뷰 해주신 alertMessage의 내용과 겹치는 것 같아 작성한 내용의 방향성이 맞는 지 여쭤보고 싶습니다.

A

Closure를 꼭 쓰지 말아야 하는것은 아닙니다. 다만 저번에 리뷰한 것 처럼 Combine의 특성
event -> Subscription 관계를 이용하면 Call back 지옥에서 벗어 날수 있기 때문입니다.

내용의 방향성이 맞는 지 여쭤보고 싶습니다. -> 이게 어떤 말인지 잘 이해가 되지 않았습니다. 다시한번 설명해 주실 수 있으일까요? 😀

pay-napster-x · 2021-04-29T05:36:20Z

https://medium.com/flawless-app-stories/you-dont-always-need-weak-self-a778bec505ef

한번 읽어보세요! Closure와 메모리 관계입니다.

eeeesong · 2021-04-29T08:59:19Z

오늘도 자세하게 리뷰해주셔서 감사합니다! 좋은 글까지 추천해주셔서 감동입니다😆
error나 optional 처리가 전반적으로 미숙한데, 최대한 고쳐보도록 노력하겠습니다ㅠㅠ

Closure를 꼭 쓰지 말아야 하는것은 아닙니다. 다만 저번에 리뷰한 것 처럼 Combine의 특성
event -> Subscription 관계를 이용하면 Call back 지옥에서 벗어 날수 있기 때문입니다.

내용의 방향성이 맞는 지 여쭤보고 싶습니다. -> 이게 어떤 말인지 잘 이해가 되지 않았습니다. 다시한번 설명해 주실 수 있으일까요? 😀

코드의 방향성을 말한 거였는데, 지금 답변해주신 부분으로 답변이 된 것 같습니다!
그렇다면 필요한 데이터가 준비되면 repository에서 usecase를 거치지 않고 바로 viewModel(subscriber)로 publish 해주어도 괜찮은 걸까요??

eeeesong · 2021-04-29T09:10:10Z

구분하는 로직을 구현하지 못하여 -> 구분하는 로직은 아직 구현하지 못하신건가요?

이 부분은 현재 Core Data / Library/Cache(이미지path)에 저장된 데이터가 있을 경우 Network를 호출하지 않는 것까진 구현했습니다!
다만 아직 이 브랜치에 push되어 있진 않습니다. 방금 구현을 해서요 :)

--> 이 부분도 merge가 되었습니다 :)

너무 고생했습니다 :)

pay-napster-x · 2021-04-30T01:41:37Z

:) usercase를 거치지 않고 바로 publish하면 viewModel이 repository를 의존하게 되니 usercase를 거쳐야 하는게 맞아보입니다. 어떠세요?
https://tech.olx.com/clean-architecture-and-mvvm-on-ios-c9d167d9f5b3

eeeesong · 2021-04-30T02:31:39Z

:) usercase를 거치지 않고 바로 publish하면 viewModel이 repository를 의존하게 되니 usercase를 거쳐야 하는게 맞아보입니다. 어떠세요?

아하 그렇군요!!
publish도 의존성을 만들어 낸다는 걸 제대로 이해하지 못하고 있었던 것 같습니다.
단계를 뛰어 넘지 않고 call back 지옥을 줄이는 방향으로 개선해보도록 하겠습니다 :) 감사합니다!!

* Feat: Tabs 구현 * Feat: Model 레이아웃 상단 구성 * Feat: Model setImgSources추가 * add: merge * fix: merge * fix: carousel animation render fix * Test: 목업데이터로 요청해보기 setupProxy * Feat: 썸네일 클릭시 이미지 변경 * fix: 캐로셀 코드 개선 애니메이션 및 함수 분리 * Fix: 클릭시 모달창 띄우기 * add: 캐로셀 이벤트 모달 및 애니메이션 추가 * fix: 캐로셀 리팩토링 * fix: 필요없는 함수 삭제 * add: 캐로셀 커스텀 훅, upper 네비게이터, 디폴트 네비게이터 분리 * fix: 캐로셀 컨테이너 리팩토링 * Feat: 에러 이미지 처리 * fix: upper navigator를 위한 padding-top 처리 * fix: 캐로셀 모달 이벤트 꼼수 수정 * fix: useCarousel 커스텀훅 clean 함수 수정 * fix: 강제지정된 높이값 수정 * add: 메인디시 캐로셀 테스트를 위한 목업 작업 * fix: 모달 바텀 테스트 통한 작업 추가, BottomSide 조건부 렌더링 추가를 통한 리렌더링 처리 -> 향후 수정 예정 * fix: 리사이즈 디바운싱 처리 * fix: useCarousel hotfix * fix: 케로셀 width height 계산로직 에러 수정 * Fix: Hearder, Tabs 반응형으로 수정 * fix: fe_dev refactor project #48 import strategy change * Fix: 상대경로 절대경로로 수정하기 fix #48 * Fix: Card Tabs분기처리 fix #51 * Fix: error 처리 useReducer로 수정 fix #50 * Fix: key를 unique하게 수정 fix #50 * Feat: useAsync 함수 구현 fix #50 * add: 전체보기 버튼 추가 * fix: 탭 상단 라벨버튼 퍼블리싱 마진값 관련 개선 * fix: 캐로셀 offset 관련 에러 및 초기 렌더 깨지는 문제 해결 * fix: 버튼 컴포넌트 엔트리 변경 * fix: 팀 API 활용을 위한 엔드포인트 처리, 카테고리 불러오기 로직 초기작업 * Fix: useAsync 수정 * Fix: api수정하여 데이터 받는부분(변수명, split(', ') 등..) 수정 * Feat: 프론트엔드 빌드 파일 추가 * fix: Dish 관리 변경 * fix: z-index 처리 * add: 랜덤 불러오기 함수 추가 * fix: 모달 refetch관련 수정 * fix: modal refetch관련 카드 수정 * Fix: api수정되어 데이터 포맷 수정 * Fix: 모달 Bottom 간격수정, console 제거 * fix: z-index hotfix Co-authored-by: jeonyeonkyu <ehb6915@naver.com> Co-authored-by: jeonyeonkyu <jyk6915@gmail.com> Co-authored-by: Jaeuk-YOO <ryou9305@naver.com> Co-authored-by: 빰빰맨 <ppamppamman@gmail.com>

eeeesong and others added 18 commits April 25, 2021 14:06

chore: pod Alamofire 설치

278a5e1

fix: viewmodel에서 알고 있는 구체타입 프로토콜로 변경

6f53e34

프로토콜로 변경함에 따라 내부 속성을 가져오는 임시함수 선언 및 사용

fix: 전 커밋에서 누락된 파일 추가

140d657

feat: Network Model에 Alamofire 적용 (#38)

8b1b0c9

fix: cell 잘리지 않도록 변경 (#39)

5e79035

fix: 런칭특가 배지 추가 (#39)

f0ae17c

- 배지를 stack view에 넣어 유동적으로 표시할 수 있도록 수정

fix: cell 간격 조정 (#39)

5bba76f

- Footer 추가 - Image View 사이즈 조정

fix: 정상가 text에 취소선 추가 (#39)

163beba

Merge branch 'fix/ios/mainUI' of https://github.com/ghojeong/sidedish …

7354adc

…into fix/ios/mainUI

feat: Repository 영역 초기화 (#31)

18f75d1

CoreData 추가, Repository 클래스 추상화

Merge pull request #45 from ghojeong/feature/ios/alamofire

9d60837

굳 :)

feature: Network Core Data 연동 테스트 (#31)

811e1c1

Merge pull request #46 from ghojeong/fix/ios/mainUI

51da168

정말 굿입니다! :)

fix: Core Data Entity 및 흐름 수정(#31)

5538734

fix: completion handler로 흐름 제어

8d5c546

fix: section id 순서대로 표시하도록 수정

7766203

chore: 불필요한 메소드 제거 (#31)

e0dd1af

Merge pull request #53 from ghojeong/feature/RepositoryCombine

1ac3f10

고생했어요 :)

Settpark changed the title ~~[team-09][Lollo, ZG] 데이터 받아서 persistant(CoreData)에 저장하고 불러오기~~ [team-09][Lollo, ZG] 데이터 받아서 persistent(CoreData)에 저장하고 불러오기 Apr 28, 2021

Settpark changed the title ~~[team-09][Lollo, ZG] 데이터 받아서 persistent(CoreData)에 저장하고 불러오기~~ [team-09][Lollo, ZG] 데이터 받아서 persistence(CoreData)에 저장하고 불러오기 Apr 28, 2021

ghojeong requested a review from pay-napster-x April 28, 2021 08:41

ghojeong added the review-iOS iOS 리뷰 label Apr 28, 2021

ghojeong assigned Settpark and eeeesong Apr 28, 2021

eeeesong added 2 commits April 28, 2021 23:11

feat: main 썸네일 caching 구현 (#55)

38502c2

refactor: 중복 메소드 정리

73fdb73

pay-napster-x reviewed Apr 29, 2021

View reviewed changes

fix: Decoding 오류 해결 (#55)

a6cc272

Merge pull request #57 from ghojeong/feature/ImageCaching

9f6ed55

너무 고생했습니다 :)

Settpark closed this Oct 2, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[team-09][Lollo, ZG] 데이터 받아서 persistence(CoreData)에 저장하고 불러오기 #48

[team-09][Lollo, ZG] 데이터 받아서 persistence(CoreData)에 저장하고 불러오기 #48

Settpark commented Apr 28, 2021

pay-napster-x left a comment •

edited

Loading

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

pay-napster-x Apr 29, 2021

eeeesong Apr 29, 2021

pay-napster-x Apr 30, 2021

pay-napster-x commented Apr 29, 2021 •

edited

Loading

pay-napster-x commented Apr 29, 2021

eeeesong commented Apr 29, 2021 •

edited

Loading

eeeesong commented Apr 29, 2021 •

edited

Loading

pay-napster-x commented Apr 30, 2021

eeeesong commented Apr 30, 2021

		}

		self.eventBadge.isHidden = badge.contains(BadgeType.event.description) ? false : true

[team-09][Lollo, ZG] 데이터 받아서 persistence(CoreData)에 저장하고 불러오기 #48

[team-09][Lollo, ZG] 데이터 받아서 persistence(CoreData)에 저장하고 불러오기 #48

Conversation

Settpark commented Apr 28, 2021

작업내용

고민 거리

pay-napster-x left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pay-napster-x commented Apr 29, 2021 • edited Loading

Q

A

pay-napster-x commented Apr 29, 2021

eeeesong commented Apr 29, 2021 • edited Loading

eeeesong commented Apr 29, 2021 • edited Loading

pay-napster-x commented Apr 30, 2021

eeeesong commented Apr 30, 2021

pay-napster-x left a comment •

edited

Loading

pay-napster-x commented Apr 29, 2021 •

edited

Loading

eeeesong commented Apr 29, 2021 •

edited

Loading

eeeesong commented Apr 29, 2021 •

edited

Loading