-
Notifications
You must be signed in to change notification settings - Fork 0
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
FIXME: 코드 1차 수정 #77
FIXME: 코드 1차 수정 #77
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
빌드가 안됩니다! 빌드가 되는 상태로 푸시해주시면 다시 화긴해볼게요!! @greetings1012
struct SolvedQuizClient { | ||
var fetch: (Int) async -> Result<[SolvedQuizEntity], D3NAPIError> | ||
} | ||
|
||
extension SolvedQuizClient: TestDependencyKey { | ||
static let previewValue = Self( | ||
fetch: { _ in .failure(.none) } | ||
) | ||
|
||
static let testValue = Self( | ||
fetch: unimplemented("\(Self.self).fetch") | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
client를 또 추가할 필요가 없는 이유는 Quiz라는 도메인으로 묶어서 생각할 수 있기 때문이라고 생각합니다. 기존에 있는 QuizClient에 fetchSolved를 하는 방식이 응집도가 높아서 더 좋아보입니다.
public enum SolvedQuizService { | ||
case fetch | ||
} | ||
|
||
extension SolvedQuizService: TargetType { | ||
public var baseURL: URL { URL(string: Environment.baseURL)! } | ||
|
||
public var path: String { | ||
switch self { | ||
case .fetch: return "quiz/list/solved" | ||
} | ||
} | ||
|
||
public var method: Moya.Method { | ||
switch self { | ||
case .fetch: return .get | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
비슷한 피드백으로 QuizService에 들어가야할 내용이라고 생각합니다. 그리고 이미 들어가있는것으로 알고있습니다!
흠... 다시 생각해보니 많은 부분들이 이미 존재하는 quizclient와 quizservice에서 가져올 수 있는 부분들이네요. branch를 다시 만들어서 처음부터 다시 고민해보겠습니다! |
PR 요약
📌 변경 사항
✅ PR check list
Linked Issue
close #76