[#191] Storage 객체 구조화 & Data Read 흐름 구축 by eeeesong · Pull Request #192 · SinsaStation/FullStackCodingBot

eeeesong · 2021-09-14T07:59:47Z

변경 사항

[Refactor] Storage 추상화 개선 #191 에 작성한 구조도를 바탕으로 객체 신설, 의존관계를 정의했습니다.
- 참고) FirebaseManager / CoreDataManager를 관리하는 BackUpCenter 객체를 추가하였습니다
MainViewModel에 산발적으로 포진되어 있던 저장 데이터 로드 흐름을 Storage 및 BackUpCenter 객체 내부로 이동했습니다.
일부 객체들의 이름을 변경하고, 디렉토리 구조를 체계화하였습니다

Storage를 제외한 신설 하위 타입들의 경우 메소드의 실제 기능은 아직 구현하지 않았습니다.

기타

현재 파베/코데 중 어디서 데이터를 불러오든 NetworkDTO 타입을 전달하는 것으로 작성이 되어있습니다
전달하는 데이터 타입이 통합되어야 객체 추상화가 가능할 것 같습니다
다만, NetworkDTO를 그대로 쓰는 것이 맥락 상 부자연스러우므로, 파베/코데의 데이터를 일괄적으로 감쌀 수 있는 새 Entity 타입을 신설하게 될 것 같습니다

- 데이터 관리 기능을 총괄하는 Storage 객체 신설 - #191에 작성한 구조에 따라 필요한 인터페이스 구축 - 세부 기능은 구현 전, Fake 객체를 사용하여 흐름만 제어 - MainViewModel에서 Auth 인증 후 Data 불러오는 코드 대폭 수정

eeeesong · 2021-09-14T08:00:39Z

@torch-ray

수정양이 많아지는 것 같아서 이대로 구동되진 않지만 우선 PR을 올립니다
머지는 하지 마시고 open 상태로 둬주시면 좋을 것 같아요!
내일 이어서 커밋하겠습니다

eeeesong · 2021-09-14T08:01:41Z

        GKLocalPlayer.local.authenticateHandler = { [unowned self] gcViewController, error in

+            guard !self.firebaseDidLoad.value else { return }
+


앞서 슬랙으로 여쭤봤던 로드됐는지 체크하는 구문은 여기로 자리를 옮겼습니다.

메서드명 변경이 필요할 것 같네요 ㅎㅎ 물론 제가 짠 코드지만요 ㅋㅋㅋ
그리고 파이어베이스 데이터 로드 완료 체크를 로그인 메서드에서 하는게 역할 분리 개념적으로 적절할까요? 만약 반드시 여기서 확인하는게 좋다면 firebaseDidLoad 프로퍼티 명을 수정하는 것도 좋은 방법일거 같긴합니다!

그렇네요ㅎㅎ 이름 변경이 필요할 것 같습니다. dataDidLoad 정도로 변경하면 괜찮을까요?

그리고 데이터 로드 체크 완료는 여기서 하는 것보다, 각 로그인 인증 단계에서 호출하는 데이터 로드 메소드에서 하는 것이 더 적절할 것 같네요!
게다가 추후 Apple Game Center 로그인 관련한 책임만 따로 짊어질 객체를 만들기로 계획했던 것 같아서
그렇게 된다면 더더욱 옮기는 편이 좋아 보입니다

firebaseDidLoad 부분은 여기서가아닌 데이터 로드 메소드에서 체크한다면 프로퍼티 명 변경은 필요 없을 수도 있을 것 같습니다!
메서드명 변경은 setupAppleGameCenterLogin() 을 말한 거였는데... 생각해보니 적절한 거 같기도 하고 ...
일단 생각하신 방향대로 진행하시면 될 것 같습니다:)

torch-ray · 2021-09-14T09:39:33Z

구조 변경이 젤 어려운데 수고하셨습니다.
저는 내일쯤 pr 올려볼게요.
RealDataStorage에 GameStorageType만 있고, GameStorage는 안보이네용? PersistenceStorage가 다 채택하게 되는 건가용? ㅎㅎ

eeeesong · 2021-09-14T09:47:56Z

@torch-ray
넵 감사합니다. 천천히 올려주세요!
PersistenceStorage의 기능들을 GameStorage와 CoreDataManager로 분화할 예정인데,
아직 그부분은 실행하지 못해서 GameStorage를 생성하지 못했습니다!
그래서 우선 GameStorageType 파일에 FakeGameStorage를 생성해서 진행했습니다.
이부분 내일 변경해서 다시 push할게요

torch-ray · 2021-09-14T10:01:45Z

넵넵 고생하셨습니다! 쉬세요:)

torch-ray · 2021-09-15T02:20:52Z

    let sceneCoordinator: SceneCoordinatorType
    let storage: PersistenceStorageType
-    let database: DatabaseManagerType
+    let database: FirebaseManagerType


기존 DatabaseManager 명을 모두 Firebase로 변경하신 것 같습니다.
저도 매우 좋은 수정방향 같은데, 근데 그렇게되면 database라는 프로퍼티나 파라미터명도 모두 firebase로 수정되는게 좋지않을까? 생각이 들긴합니다.

넵! firebase로 통일하여 수정하겠습니다

torch-ray · 2021-09-15T02:23:33Z

+
+    func load(with uuid: String?, _ isFirstLaunched: Bool) -> Observable<NetworkDTO> {
+        Observable<NetworkDTO>.create { [unowned self] observer in
+            if isFirstLaunched {


isFirstLaunched가 true면 여기서 코어데이터 create호출하고 바로 메서드 종료되어야 하는 거 아닌가요?

다른 기기에서 로그인 이력이 있다면 확인해야돼서 uuid 체크 후 firebase 데이터 연동이 필요합니다!

torch-ray · 2021-09-15T02:24:49Z

+        self.backUpCenter = backUpCenter
+    }
+
+    func fill(using uuid: String?, isFirstLaunched: Bool) -> Observable<Bool> {


fill이 스토리지를 채우다라는 뜻인가요? using uuid 파라미터를 보면 uuid를 활용해서 채운다는 뜻 같은데, 메서드 이름이 조금 더 자세하면 좋겠습니다.

넵 스토리지를 채운다는 말이 맞긴 한데
조금 더 직관적인 작명 고민해보겠습니다!

- BackUp과 관련한 기능으로 판단

- 기존 PersistenceStorage의 책임을 옮겨 옴

- ViewModel 이니셜라이저 변경 - 새 객체들의 인터페이스 정의 - 임시 메소드 생성

- 각 객체의 저장 기능을 연결지었다

- 빠진 구독 추가

eeeesong · 2021-09-15T08:31:23Z

@torch-ray

이틀차 리팩토링 commit들 올려두었습니다~!
어제 논의한 대로 구조에 맞춰서 객체 역할을 나눴고, 정상 구동하는 것까지 확인했습니다.
(일반: 네트워크 있는 상황/없는 상황, 앱 첫 구동: 네트워크 있/없)
미처 확인 못한 부분이 있을 수 있으니 한번 확인 부탁드려요!

진행하면서 추가적으로 발견한 리팩토링 할 만한 사항들도 공유합니다

StorageType의 인터페이스 분리 & 개선

Storage의 메소드가 쓰이는 곳을 확인해보니 아래와 같았습니다. 각 ViewModel에 필요한 인터페이스만 가지고 있게 할 수도 있지 않을까 하는 생각이 들어서 코멘트를 유지해두었습니다

protocol StorageType {
    
    // Main 
    func initializeData(using uuid: String?, isFirstLaunched: Bool) -> Observable<Bool>
    
    // Shop
    func availableRewards() -> Observable<[ShopItem]>
    func availableMoney() -> Observable<Int> // + Item
    func setNewRewardsIfPossible() -> Observable<Bool>
    func rewardNeedsToBeGiven(with finishedAd: GADRewardedAd?) -> Int
    
    // Item
    func itemList() -> [Unit]
    func raiseLevel(of unit: Unit, using money: Int) -> Unit
    
    // Game
    func myHighScore() -> Int
    
    // GameOver
    func raiseMoney(by amount: Int)
    func updateHighScore(new score: Int) -> Bool
    
    // Background
    func save()
}

그리고 어떤 곳에서는 Observable을 리턴하고 또 어디서는 값만 리턴하는데 적당한 기준에 따라서 통일하는 것이 좋을 것 같습니다.
동기적으로 작동하는 경우에는 일반 값으로 통일하면 어떨까요?

DataFormatManager

어제 잠깐 논의를 한 부분이긴 한데, 고치면서 보니까 BackUp 관련된 부분에서만 쓰여서 굳이 Static이여야 하는지에 대한 의문이 들었습니다.
FirebaseManager와 CoreDataManager가 각각 필요한 부분만 가지고 있게 하는건 어떨지...

상수 정의하기

BackUp 관련한 Key 값들은 전부 상수로 저장해놓으면 좋을 것 같습니다
각 객체에서 필요한 부분을 내부적으로 정의해서 쓰면 되지 않을까 싶어요!

torch-ray · 2021-09-16T01:43:42Z

StorageType의 인터페이스 분리 & 개선

말씀하신 대로 세부적으로 분리가능하도록 개선해보는 게 좋을 것 같습니다. 방법은 조금 더 생각해봐야겠네요.
Observable 값 통일도 앱 제작 당시에 고려했던 부분인데, 값 자체를 리턴하는 메서드가 반드시 필요해서 사용했던 걸로 기억이 나네요. 기준이 애매하다고 생각했는데 동기적으로 통작하는 코드는 Observable을 제거하는 쪽으로 방향을 잡아볼까요?

DataFormatManager

아예 해당 객체를 없애고, 메서드를 각각 다른 객체로 분리한다는 말씀이신가요?

상수 정의하기

넵, 그게 좋을 것 같습니다.

torch-ray · 2021-09-16T02:06:45Z

            }

-            firebaseManager.getFirebaseData(uuid)
+            firebaseManager.load(uuid)


load만 있으니 헷갈리네요. 이럴 경우엔 파라미터 internal, external name을 같이 쓰는게 나을거 같습니다. load(with uuid: String) 이런식으로 한다면, 호출할 때 firebaseManager.load(with: uuid) 이렇게 되고 가독성이 좀 더 나은 느낌입니다.

매우 동의합니다!
파라미터 네이밍을 통해 이해도를 올려보도록 하겠습니다

torch-ray · 2021-09-16T02:08:07Z

    }

-    func updateDatabase(_ info: NetworkDTO) {
+    func save(_ info: NetworkDTO) {


load때와 마찬가지로 파라미터 name을 생략하기 보다 더 적절한 external name을 생각해봐야 할 것 같습니다. 메서드명이 명확한 경우엔 생략이 좋을 수 있는데 , load save는 파라미터 name이 있으면 조금 더 이해가 쉬울 것 같습니다 ㅎㅎ

eeeesong · 2021-09-18T02:20:20Z

StorageType의 인터페이스 분리 & 개선

말씀하신 대로 세부적으로 분리가능하도록 개선해보는 게 좋을 것 같습니다. 방법은 조금 더 생각해봐야겠네요.

Observable 값 통일도 앱 제작 당시에 고려했던 부분인데, 값 자체를 리턴하는 메서드가 반드시 필요해서 사용했던 걸로 기억이 나네요. 기준이 애매하다고 생각했는데 동기적으로 통작하는 코드는 Observable을 제거하는 쪽으로 방향을 잡아볼까요?

인터페이스 분리: 넵 회의 전까지 방법 생각해보겠습니다.
Observable: 이 부분도 한번 각자 생각해보고 논의를 해보면 좋을 것 같습니다. 다시 생각해보니 동기적으로 동작하는 코드이더라도 실패할 가능성이 있다면 Observable를 리턴하는 것이 의미가 있을 수 있겠다는 생각이 드네요..!

DataFormatManager

아예 해당 객체를 없애고, 메서드를 각각 다른 객체로 분리한다는 말씀이신가요?

넵 그런 의미가 맞습니다.
아니면 혹시 조금 더 추상화를 더하는 방향으로, CoreDataManager와 FirebaseManager의 공통 인터페이스를 만들고,
필요한 DataFormatManager (or 메소드 단위?)를 주입해줄 수 있는 주입 메소드를 더하는 방법도 괜찮을 것 같습니다!

상수 정의하기

넵, 그게 좋을 것 같습니다.

이 부분은 금요일 회의 전까지 수정 해놓겠습니다!

eeeesong added 2 commits September 14, 2021 16:43

refactor: [#191] 객체 이름 변경, 디렉토리 체계화

2a2997a

eeeesong added refactoring refactoring song labels Sep 14, 2021

eeeesong added this to the 1.05 milestone Sep 14, 2021

eeeesong requested a review from torch-ray September 14, 2021 07:59

eeeesong self-assigned this Sep 14, 2021

eeeesong commented Sep 14, 2021

View reviewed changes

torch-ray reviewed Sep 15, 2021

View reviewed changes

eeeesong added 13 commits September 15, 2021 13:11

refactor: [#191] 메소드 이름 변경

bb411c9

refactor: [#191] 쓰지 않는 프로퍼티 삭제

927dc5b

refactor; [#191] DataFormatManager 디렉토리 이동

1b98ac1

- BackUp과 관련한 기능으로 판단

refactor: [#191] CoreData 관련 기능만 수행하는 객체 신설

06fea17

refactor: [#191] Game 데이터 관리 객체 신설

d8fe093

- 기존 PersistenceStorage의 책임을 옮겨 옴

refactor: [#191] PersistenceStorage 삭제에 따른 변경

5c5e9f3

- ViewModel 이니셜라이저 변경 - 새 객체들의 인터페이스 정의 - 임시 메소드 생성

refactor: [#191] Storage 객체 이니셜라이저 디폴트 값 추가

7a98e1c

refactor: [#191] 저장 흐름 연결

e7e4f50

- 각 객체의 저장 기능을 연결지었다

refactor: [#191] 불필요한 메소드 제거, 실수로 지운 메소드 추가

49093f5

fix: [#191] 리워드 동기화 문제 해결

0c29d77

- 빠진 구독 추가

chore: [#191] 오타, 코멘트 수정

1949c99

fix: [#191] 빠진 저장 구문 추가

20082ba

refactor: [#191] 프로퍼티 이름 변경, 호출 위치 변경

b4ba872

eeeesong changed the title ~~[Refactor 1-1] Storage 객체 구조화 & Data Read 흐름 구축~~ [#191] Storage 객체 구조화 & Data Read 흐름 구축 Sep 15, 2021

torch-ray reviewed Sep 16, 2021

View reviewed changes

eeeesong added 2 commits September 23, 2021 13:50

refactor: [#191] Storage Key 값 상수 정의

264a8d4

refactor: [#191] 가독성 개선을 위한 매개변수 명칭 변경

4999cf0

torch-ray merged commit ae4558c into iOS/dev Sep 24, 2021

		GKLocalPlayer.local.authenticateHandler = { [unowned self] gcViewController, error in

		guard !self.firebaseDidLoad.value else { return }

Uh oh!

Conversation

eeeesong commented Sep 14, 2021

변경 사항

기타

Uh oh!

eeeesong commented Sep 14, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

torch-ray commented Sep 14, 2021

Uh oh!

eeeesong commented Sep 14, 2021

Uh oh!

torch-ray commented Sep 14, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eeeesong commented Sep 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

torch-ray commented Sep 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eeeesong commented Sep 18, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eeeesong commented Sep 15, 2021 •

edited

Loading