-
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
Feat/#3 network layer #7
base: main
Are you sure you want to change the base?
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.
PR에 남겨주신 질문에 대해서는 추후 기능에 따른 API 항목을 분리하는 것이 나을 것 같습니다.
꼼꼼히 API 명세를 읽어본 것은 아닌데, 필요하지 않다면 나누지 않고 사용해도 괜찮습니다. 계속 논의 해봐요!
@@ -265,7 +275,7 @@ | |||
CODE_SIGN_STYLE = Automatic; | |||
CURRENT_PROJECT_VERSION = 1; | |||
DEVELOPMENT_ASSET_PATHS = "\"Staccato-iOS/Preview Content\""; | |||
DEVELOPMENT_TEAM = G7X65ZSJ3J; | |||
DEVELOPMENT_TEAM = CHW94B5XTB; |
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.
충돌 이슈가 있겠군요!
import Foundation | ||
|
||
struct ImageURL: Codable { | ||
let ImageURL: String // 이미지 URL |
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.
LowerCamelcase를 지켜서
let ImageURL: String // 이미지 URL | |
let imageURL: String // 이미지 URL |
로 사용하는 것은 어떨까요?
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.
Response 혹은 Request가 저 형태로 되어야 한다면, CodingKey를 사용해야 할 것 같습니다!
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.
import Foundation | ||
|
||
struct Comment: Codable { | ||
let commentId: Int64 // 댓글 ID |
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.
Int64를 사용하면, 다른 연산작업이 조금 어렵지 않을까요? 매번 Type Casting을 해주어야 할 것 같은데, Int64를 사용하는 것이 Int를 사용하는 것에 비한 다른 이점이 있을까요?
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.
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.
어짜피 Jason 형태로 Response 받을 거라 Type Parsing은 필요에 의해 우리가 정하면 될 것 같은데, 서버의 요구 사항이 64bit로 받아야 한다는 전제가 있을까요?
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.
아 그런가요! 🤔🤔
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.
이외에도 추가적으로 서버에서 제공하는 Error 타입이 명시되어야 한다고 생각합니다. 이건 추후 작업하면서 계속 늘리면 될 것 같아요!
import Alamofire | ||
|
||
final class NetworkService { | ||
static let shared = NetworkService() |
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.
인스턴스화 해서 Singleton으로 하는게 나을까요? 혹은 여기에 정의되는 메서드들을 Type Method로 정의해서 사용하는 것이 나을까요?
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.
크게 상관은 없는데 나중에 Type method로 하면 나중에 테스트용으로 Mocking할때 override가 안되어서 저는 보통 저렇게 했습니다 의논해보죠
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.
데이터 양이 많았을텐데 고생하셨습니다!!!👍👍👍
import Foundation | ||
|
||
struct Comment: Codable { | ||
let commentId: Int64 // 댓글 ID |
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.
import Foundation | ||
|
||
struct ImageURL: Codable { | ||
let ImageURL: String // 이미지 URL |
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.
머지 Base branch가 main으로 되어있네요!!! develop으로 수정해주심 감사하겠습니다! |
⭐️ Issue Number
🚩 Summary
🙂 To Reviewer
모델은 안드로이드랑 동일하게 맞췄으나 추후에 변경되거나 제가 실수한 부분이 있을 수도 있어서 기능 구현할시 디코딩 오류가 날 수도 있습니다.
현재 APIEndPoint에서 API를 전체를 관리하도록 되어있는데 이걸 Memory, Moment, Login 등등 나누는게 어떨까 고민되어서 의논하면 좋을 것 같습니다.
📋 To Do
나머지 API 추가