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

[Eddy] Firestore 관련 요청을 타입과 레퍼런스와 상관없이 Generic하게 수정 #54

Merged
merged 5 commits into from May 19, 2022

Conversation

BumgeunSong
Copy link
Owner

📙 작업 내역

  • Entity를 파일 분리
  • FirestoreRequest 프로토콜, UserRequest 타입을 추가

🤔 고민과 해결

  • Firestore service를 addService, getService로 나누려다가, 그것보다는 우리가 요청하는 다양한 타입과 리소스를 직접 타입으로 추가하여 generic하게 실행할 수 있도록 만드는 것이 더 좋다고 생각하여 리팩토링을 시도함.
  • 아직 해결되지 않은 문제는, reference와 data까지는 제네릭하게 만들었는데, Query나 Save의 다양한 케이스들을 다 커버하지 못함. Task-Save의 경우는 array에 추가하는 함수를 사용하며, Boards-Query의 경우는 getDocument가 아닌 getDocuments를 써야함. 또는 커스텀 객체를 넘겨 저장할 때와 딕셔너리로 넘겨서 저장하는 케이스도 있음.
  • 이처럼 다양한 쿼리와 저장의 케이스가 있기 때문에, 실제 요청 함수와, 응답 처리 함수도 generic하게 만들어야 하는데, 그 부분을 어떻게 해야할지 감이 잘 오지 않음.
  • Generic하게 네트워킹 레이어를 만드는 예제를 참고해서 계속 고민해볼 예정

💭 기타 설명

@BumgeunSong BumgeunSong reopened this May 18, 2022
@BumgeunSong BumgeunSong changed the base branch from release to dev May 18, 2022 09:15
@BumgeunSong BumgeunSong self-assigned this May 18, 2022

typealias FirestoreUID = String
typealias FireabaseAuthUID = String

Copy link
Collaborator

Choose a reason for hiding this comment

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

🙋FirestoreUID 랑 FireabaseAuthUID 를 따로 놓는이유가 있을까요? Auth 의 uid 랑 fireStore 의 uid 는 같지 않나요?

import FirebaseFirestoreSwift

protocol FirestoreRequest {
associatedtype Modeltype: Codable
Copy link
Collaborator

Choose a reason for hiding this comment

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

associatedtype 은 아직 사용하지 않는거죵?

Copy link
Owner Author

Choose a reason for hiding this comment

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

아래 26번째 줄 UserRequest에서 사용하고 있습니다!


var docReference: DocumentReference {
db.collection("users").document(uid)
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

🔵 사용되는 Collection 은 이미 정해져있는거니 Collection reference 를 enum 으로 따로 빼 놓고 사용하는건 어떨까요?

Copy link
Collaborator

Choose a reason for hiding this comment

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

enum 으로 정의하는 것도 좋은 방법이네유!

Copy link
Owner Author

Choose a reason for hiding this comment

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

이 부분도 뭔가 enum으로 구조화할 수 있을 거 같은데, Reference의 타입이 3가지(DocumentReference, CollectionReference, Query)나 되고, 거기에 따라서 실행해야하는 메서드 이름도 다 달라서 어떻게 해야할지 머리를 쥐어짜는 중...

개선해서 다시 push 해보겠습니다.

Copy link
Collaborator

@Jinsujin Jinsujin left a comment

Choose a reason for hiding this comment

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

깔끔하게 정리정돈된 코드를 보는 것 같아 속이 편안해 집니다~
request 에 필요한 데이터를 구조체로 묶는 것 만으로 역할이 분명해 졌다고 느꼈습니다.
protocol 을 사용하니 디코딩할때 타입 변환도 편하네요 고생하셨습니다!

}
}

struct UserRequest: FirestoreRequest {
Copy link
Collaborator

Choose a reason for hiding this comment

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

각각의 Request 를 struct 로 묶는 방법 이로군요 🤔 좋은 시도인것 같습니다! 공통적으로 사용하는 Firestore.firestore() 를 extension 으로 묶은것도 좋네욧

completion(.failure(.emptyData))
return
}
guard let decodedModel = try? document.data(as: Request.Modeltype.self) else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

👏🏻👏🏻 protocol 를 채택한 모델을 이용해서 이렇게 간단하게 디코딩할 수 있네요!

@@ -23,7 +23,7 @@ extension FirestoreRequest {
}

struct UserRequest: FirestoreRequest {
typealias Modeltype = User
typealias Modeltype = UserProfile
Copy link
Collaborator

Choose a reason for hiding this comment

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

User 가 UserProfile 로 바뀌었네요! 훨씬 명확해 진 것 같습니다

@BumgeunSong BumgeunSong merged commit 0cf33b5 into dev May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants