[Feature] 화이트보드 생성 및 게시 구현 #82
Conversation
| import Foundation | ||
|
|
||
| public final class WhiteboardRepository: WhiteboardRepositoryInterface { | ||
| private var nearbyNetworkInterface: NearbyNetworkInterface |
There was a problem hiding this comment.
구체 클래스에서는 변수 명으로 interface는 생략하는건 어떤가요??
| // | ||
|
|
||
| public protocol WhiteboardUseCaseInterface { | ||
| var repository: WhiteboardRepositoryInterface { get } |
There was a problem hiding this comment.
UseCase를 정의할 때 repository에 대한 내용은 숨기는 건 어떨까요?
조이의 pr에 같은 리뷰가 있어 링크 남겨드립니다!! 확인해주시면 감사하겠습니다 :)
#79 (comment)
| // | ||
|
|
||
| public final class WhiteboardUseCase: WhiteboardUseCaseInterface { | ||
| public var repository: WhiteboardRepositoryInterface |
| } | ||
|
|
||
| public func createWhiteboard(nickname: String) -> Whiteboard { | ||
| return repository.createWhiteboard(nickname: nickname) |
There was a problem hiding this comment.
repository에 생성한 화이트보드를 전달해주는건 어떤가요?
| let createWhiteboardAction = UIAction { [weak self] _ in | ||
| self?.viewModel.action(input: .createWhiteboard) | ||
| } | ||
| createWhiteboardButton.addAction(createWhiteboardAction, for: .touchUpInside) |
| .addToSuperview(view) | ||
| .size(width: 199, height: 43) | ||
| .top(equalTo: view.topAnchor, inset: 61) | ||
| .leading(equalTo: view.leadingAnchor, inset: 22) |
There was a problem hiding this comment.
디자인 시스템에 상수로 좌우 패딩 값이 선언되어 있는 것으로 알고 있습니다.
해당 상수를 사용하는 건 어떤가요?
There was a problem hiding this comment.
가능하면, 자주 쓰이는 패딩 값도 View 코드 상단에 enum으로 관리하면 좋을 것 같습니다!
There was a problem hiding this comment.
디자인 시스템에 상수로 좌우 패딩 값이 선언되어 있는 것으로 알고 있습니다. 해당 상수를 사용하는 건 어떤가요?
조이의 PR 머지 후 적용하겠습니다!!
가능하면, 자주 쓰이는 패딩 값도 View 코드 상단에 enum으로 관리하면 좋을 것 같습니다!
동의합니다!
| } | ||
|
|
||
| struct Output { | ||
| let whiteboardSubject: PassthroughSubject<Whiteboard, Never> |
There was a problem hiding this comment.
저희 viewModel 스타일을 새로 정해서 아래 링크 참고해주시면 좋을 것 같습니다!~!
https://buttoned-package-f84.notion.site/MVVM-c120ca7c53304cdc981a74bdcb0fdfab?pvs=4
| name: String, | ||
| peers: [UUID] = [], | ||
| objects: [WhiteboardObject] = [], | ||
| chats: [String] = [] |
There was a problem hiding this comment.
peers와 chats는 아직 필요 없어 보입니다 !!
| public protocol WhiteboardRepositoryInterface { | ||
| /// 화이트보드를 생성합니다. | ||
| /// - Parameter nickname: 유저 닉네임(화이트보드의 이름으로 사용) | ||
| func createWhiteboard(nickname: String) -> Whiteboard |
There was a problem hiding this comment.
return 값에 대한 Documents 주석은 없나용 ??
| /// - Parameter nickname: 유저 닉네임(화이트보드의 이름으로 사용) | ||
| func createWhiteboard(nickname: String) -> Whiteboard | ||
|
|
||
| /// 화이트보드를 주번에 알립니다. |
| // | ||
|
|
||
| public protocol WhiteboardUseCaseInterface { | ||
| var repository: WhiteboardRepositoryInterface { get } |
There was a problem hiding this comment.
저희 아까 제 PR에서 컨벤션 정할 때 UseCase의 인터페이스에서는 repository 프로퍼티를 갖고 있지 않도록 했습니다 !!
|
|
||
| private let createWhiteboardButton: UIButton = { | ||
| let button = UIButton() | ||
| button.setTitle("", for: .normal) |
There was a problem hiding this comment.
불필요한거같아 제거하도록 하겠습니다!
| .addToSuperview(view) | ||
| .size(width: 199, height: 43) | ||
| .top(equalTo: view.topAnchor, inset: 61) | ||
| .leading(equalTo: view.leadingAnchor, inset: 22) |
There was a problem hiding this comment.
이런 숫자들 저희 ViewController의 private enum으로 정리하기로 하지 않았나요 ?
There was a problem hiding this comment.
그랬나유..?
바로 적용하도록 하겠습니다
|
|
||
| public final class WhiteboardListViewModel: ViewModel { | ||
| private let whiteboardUseCase: WhiteboardUseCaseInterface | ||
| private var nickname: String |
There was a problem hiding this comment.
닉네임을 갖고 있는 것보다 Persistence에서 Profile 꺼내서 사용하는건 어떤가요 ??
There was a problem hiding this comment.
닉네임을 변경할 때 마다 값을 전달받아 변경하는 방식보다는, 필요할 때 UserDefault에서 꺼내서 쓰는게 더 좋은거같습니다!
추천해주신 방법으로 수정해보겠습니다~~
| // | ||
|
|
||
| public final class WhiteboardUseCase: WhiteboardUseCaseInterface { | ||
| public var repository: WhiteboardRepositoryInterface |
There was a problem hiding this comment.
인터페이스에서 repository 프로퍼티를 제거하면 해당 repository 프로퍼티도 private let으로 사용할 수 있을 것 같습니다 !
| import Foundation | ||
|
|
||
| public final class WhiteboardRepository: WhiteboardRepositoryInterface { | ||
| private var nearbyNetworkInterface: NearbyNetworkInterface |
There was a problem hiding this comment.
nearbyNetwork 구현체가 클래스이기도 하고, 변화가 없는데요!
혹시 var로 선언 해 주신 이유가 있을까요?
| public func createWhiteboard(nickname: String) -> Whiteboard { | ||
| return Whiteboard(name: nickname) | ||
| } |
There was a problem hiding this comment.
createWhiteboard 메서드는 repository에서 해주신 이유가 있을까요?
repository의 역할과는 조금 멀게 느껴지는데요,
단지 화이트보드를 만들어 반환 해 주는 거라면 usecase에 있는 것은 어떨까요??
There was a problem hiding this comment.
제가 repository의 개념을 제대로 잡고있지 않았던거같습니다
말씀을 듣고보니 화이트보드 생성에 관한 함수는 UseCase에 구현하는게 맞다고 생각합니다 ㅎㅎ
| import Foundation | ||
|
|
||
| public struct Whiteboard { | ||
| let name: String |
There was a problem hiding this comment.
name의 용도가 어떻게 될지 궁금합니다!!
name의 용도가 뷰에서 whiteboard의 이름으로 사용된다면 public을 해줘야 할 것 같습니다 :)
| public protocol WhiteboardRepositoryInterface { | ||
| /// 화이트보드를 생성합니다. | ||
| /// - Parameter nickname: 유저 닉네임(화이트보드의 이름으로 사용) | ||
| func createWhiteboard(nickname: String) -> Whiteboard |
There was a problem hiding this comment.
구현체의 코멘트와 동일하게 같이 봐주시면 좋을 것 같습니다!!
| // | ||
|
|
||
| public protocol WhiteboardUseCaseInterface { | ||
| var repository: WhiteboardRepositoryInterface { get } |
There was a problem hiding this comment.
조이의 pr에도 같은 내용을 달았는데요,
프로토콜의 역할중 구현체가 해당 프로퍼티나 메서드를 갖고있다라는 의미로 해석되는 부분도 있다고 생각합니다.
레포지터리를 갖고있다는 것을 외부에 알려줄 필요가 없다고 생각합니다!!그렇기에 구현체가 가지고 있어야 할것들을 인터페이스가 정의 해준다라는 것은 역할의 역전이라고 생각이 듭니다..
고로 레퍼지터리는 인터페이스가 갖고있을 필요는 없지 않을까요?.?
참고하면 좋을 것 같습니다!!
| required init?(coder: NSCoder) { | ||
| fatalError("init(coder:) has not been implemented") | ||
| } |
| private let whiteboardUseCase: WhiteboardUseCaseInterface | ||
| private var nickname: String | ||
|
|
||
| enum Input { | ||
| case createWhiteboard | ||
| } | ||
|
|
||
| struct Output { | ||
| let whiteboardSubject: PassthroughSubject<Whiteboard, Never> | ||
| } | ||
|
|
||
| var output: Output |
There was a problem hiding this comment.
프로퍼티와 Input Output의 순서도 다같이 얘기해 보면 좋을 것 같아요 :)
taipaise
force-pushed
the
feature/CreateWhiteboard
branch
from
cd4d7be to
bcec5a8
Compare
🌁 Background
화이트보드 생성 및 게시를 구현했습니다.
📱 Screenshot
👩💻 Contents
✅ Testing
📝 Review Note
📣 Related Issue