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

refactor: 레포지토리 리펙토링 #175

Closed
wants to merge 21 commits into from
Closed

refactor: 레포지토리 리펙토링 #175

wants to merge 21 commits into from

Conversation

Sonny-Kor
Copy link
Collaborator

What is this PR?

  • 상위/하위 레포지토리 Refactoring
  • 로그킷을 통해 DEBUG 상태에서 Network 요청 정보들을 디버깅할 수 있도록 Entitiy에 description 추가

PR Type

  • Bugfix
  • Chore
  • New feature (기능을 추가하는 feat)
  • Breaking change (기존의 기능이 동작하지 않을 수 있는 fix/feat)
  • Documentation Update

Further comments

Main Repository에서 모든 DB의 프로퍼티의 변경 사항을 하위 레포지토리에 send하는 문제가 발생했었습니다.
그로 인해 모든 하위 레포지토리는 2가지 이상의 변경 사항이 있는 경우 (ex Round와 RecordOrder가 동시에 변경하는 둥) 하위 레포지토리 에서는 Zip이나 CombineLatest와 같이 동시에 Publisher들을 묶었어야 했고, 분기처리가 필요했습니다. 서버에서 Round와 RecordOrder를 (0 0)으로 바꾸면, Swift에서는 CombineLatest는 (nil, 0) (0 0) 로 들어오기때문, Zip의 경우도 사용하기 불편하였습니다.

변경된 MainRepository

public protocol MainRepositoryProtocol {
    var myId: String? { get }
    var room: CurrentValueSubject<Room?, Never> { get }

    func connectRoom(roomNumber: String)
    func disconnectRoom()

    func createRoom(nickname: String, avatar: URL) async throws -> String
    func joinRoom(nickname: String, avatar: URL, roomNumber: String) async throws -> Bool
    func leaveRoom() async throws -> Bool
    func startGame() async throws -> Bool
    func changeMode(mode: Mode) async throws -> Bool
    func changeRecordOrder() async throws -> Bool
    func resetGame() async throws -> Bool
    func submitAnswer(answer: Music) async throws -> Bool
    func getAvatarUrls() async throws -> [URL]
    func getResource(url: URL) async throws -> Data
    func submitMusic(answer: ASEntity.Music) async throws -> Bool
    func postRecording(_ record: Data) async throws -> Bool
}

이번 PR의 리펙토링 변경사항은 크게 2가지가 있습니다.

  1. 상위레포에서는 room 하나의 프로퍼티만 send하고, 하위 레포지토리에서는 room을 구독하게됩니다.

이전의 데이터와 중복 값을 가지는 경우에 값을 방출 하지 않도록 removeDuplicates operator 를 활용했습니다.

  1. 하위 레포지토리에서는 NetworkKit을 사용하지 않는다.

걱정인 부분은 2번 입니다. 하위 레포지토리에서 Network Kit을 사용하지않고, 상위 레포지토리에서 네트워크 처리가 들어가다 보니, 반복적인 코드가 늘어나고, API가 늘어날수록 더 커질 수 있을 것 같습니다.

  • 해결 책이 떠오르지 않아서 우선 레포지토리 리펙토링 한 부분만 PR을 작성했습니다. 다른분들의 의견도 궁금합니다.

잡다한 커밋

  • 개발 중 iOS 17 버전에서 microphone symbol이 안보이는게 보여서 분기처리했습니다.
if #available(iOS 18.0, *) {
    (systemName: "microphone", color: "FD5050")
} else {
    (systemName: "mic", color: "FD5050")
}

로그 킷 사용기
로그를 보는데 항상 BreakPoint를 찍고 확인하는 과정이 불편해서 로그를 추가했습니다.

  • 네트워크 요청
  • 변경된 Room
기능 스크린샷
로그킷을 사용해서 로 쉽게 디버깅 확인

@Sonny-Kor Sonny-Kor added the refactor 새로운 기능/버그 수정 없이 코드 모양만 변경 label Dec 2, 2024
@Sonny-Kor Sonny-Kor self-assigned this Dec 2, 2024
@Sonny-Kor Sonny-Kor linked an issue Dec 2, 2024 that may be closed by this pull request
Comment on lines +153 to +157
if #available(iOS 18.0, *) {
(systemName: "microphone", color: "FD5050")
} else {
(systemName: "mic", color: "FD5050")
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

버전이 바뀌면서 Symbol 명이 바뀌었나 보군요 ㄷㄷㄷ

.receive(on: DispatchQueue.main)
.compactMap { $0?.status }
.removeDuplicates()
Copy link
Collaborator

Choose a reason for hiding this comment

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

이전 값과 비교하여 값을 방출할지 말지를 정하는 메서드군요

Copy link
Collaborator

@Tltlbo Tltlbo left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!
removeDuplicates operator가 이전 값과 비교하여 변경된 값만 방출하는 걸로 알고 있는데
기존에도 그런 처리를 해줬는데 왜 차이가 발생할까요?

@Sonny-Kor
Copy link
Collaborator Author

고생하셨습니다! removeDuplicates operator가 이전 값과 비교하여 변경된 값만 방출하는 걸로 알고 있는데 기존에도 그런 처리를 해줬는데 왜 차이가 발생할까요?

아무래도 동시 에 프로퍼티가 변경됐을 때 하위레포가 2번 받게되는 문제가 큰거같아요

Copy link
Collaborator

@psangwon62 psangwon62 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!
pr 전 rebase pull은 꼭 해주세용

@Sonny-Kor Sonny-Kor closed this Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor 새로운 기능/버그 수정 없이 코드 모양만 변경
Projects
None yet
Development

Successfully merging this pull request may close these issues.

네트워크 통신 리펙토링
4 participants