AI 챗봇 앱 [STEP 3] Matthew, L #49
Conversation
There was a problem hiding this comment.
Mattew, L 너무 수고하셨습니다!
Rx를 적용해 보시려고 노력하신 부분 너무 좋습니다!
제가 리뷰로 달고 싶은 부분들을 다 질문에 써주셔서 질문에 대한 답변을 위주로 하면 될 것 같습니다 😗
✨ ViewModel 활용
현재 저희 로직상 ViewModel을 두고 있지만 대부분의 기능을 지금 ViewController에서 하고 있습니다. 아직도 MVC의 C의 의존을 버리지 못한게 아쉽네요 ㅠ 보다 더 활용을 할 수 있는 방법이 있을까요?
현재 뷰랑 연결되어서 데이터에 대한 세팅이나 설정을 모두 ViewController에서 하고 있습니다.
저희 로직중에 ViewModel로 빼거나 뺄수 있는 부분에 대해 말씀해주시면 감사하겠습니다!
지금, messageList 자체를 VM으로 옮긴다고 생각하시면, 어떤 방향으로 수정하셔야 할지 감이 오실 것 같습니다.
구조에 대해서는 이렇다라고 할 정답이 있는 것은 아니지만, 현재는 VM에서 정말 데이터 호출만 담당하고 있어서 조금 아쉬운 것 같아요.
대부분의 설정을 VM에서 할 수 있도록 개선해보시고, VC가 필요한 정보들을 VM에서 가져와서 세팅하는 쪽으로 생각해보세요!
의존성을 분리하는 것이 너무 어렵다면, 지금은 Input Output으로만 소통하기로 규칙을 정하신 것 같지만, vc가 vm을 가지고 있는 형태이다 보니, viewModel. 이런식으로 접근 할 수 있다 라는 가능성도 열어두고 생각해보세요!
✨ Network 계층
RxAlamofire를 사용하여 network 통신을 하고 있는데 현제 로직이
Network → NetworkProvider → ChatBotNetwork 이러한 계층을 갖고 있는데 이렇게 나누는게 맞는지 그리고 각각의 나눈 역할에 대한 장점을 잘 활용하고 있는지 의문입니다.
ChatBotNetwork에 대한 Mock Test를 진행하면, 계층을 분리하신 장점을 더욱 살릴 수 있을 것 같습니다.
지금은 ChatBotNetwork 밖에 없어서, 계층을 나눈 장점이 잘 보이지는 않지만, 다양한 API 들이 추가 되는 경우에는 확실한 역할 분리의 장점을 느끼실 수 있을 것 같습니다 😀
✨ Trigger OnNext 활용
Trigger를 통해 bind 하는 과정에서 onNext로 들어오는 데이터를 제대로 활용하지 못하는것 같아서 많이 아쉬웠습니다.
그냥 지금은 전에 델리게이트 처럼 통신하고 그 데이터를 갖다 쓰는것 뿐이지 제대로 Rx를 통해 OnNext에서 처리하는게 없어서 이러한 부분에 대한 방향성이 필요합니다!
func bindView() {
chatInputTextView.enterButton.rx.tap.bind { [weak self] _ in
guard
let userInputText = self?.chatInputTextView.inputTextField.text
else {
return
}
let userMessage = Message(role: "user", content: userInputText)
self?.chatTrigger.onNext(userMessage)
self?.messageList.append(userMessage)
self?.chatList.reloadData()
}.disposed(by: disposeBag)
}onNext를 꼭 활용해야 하는 것은 아닙니다!
델리게이트와 비슷하게 사용해도 된다고 생각합니다. 굳이 처리를 하지 않아도 된다고 생각하면요.
지금 위에 작성해주신 코드에서 rx의 특징을 조금 더 활용해 보고 싶다면,
func bindView() -> Observable<Message> {
chatInputTextView.enterButton.rx.tap
.flatMap { [weak self] _ -> Observable<Message> in
guard let self = self,
let userInputText = self.chatInputTextView.inputTextField.text, !userInputText.isEmpty else {
return Observable.empty()
}
let userMessage = Message(role: "user", content: userInputText)
return Observable.just(userMessage)
}
.do(onNext: { [weak self] message in
self?.chatTrigger.onNext(message)
})
.disposed(by: disposeBag)
}이런식으로 flatMap을 활용해서 데이터를 바꾸는 방향도 있을 것 같아요.
✨ 라이브러리 추천
라이브러리를 더 다양하게 사용을 못한것 같아 아쉬웠습니다.
현직에서 자주 사용 되고 이외에 추가적으로 더 공부하면 도움이 될거 같은 라이브러리를
추천해주시면 감사하겠습니다~
지금 사용하시는 라이브러리들이 자주 사용하는 라이브러리들이라서 잘 사용하고 계신 것 같습니다.
나중에 이미지 처리 하실 때 Kingfisher 사용해보시길 추천 드릴께요!
나머지 사항들은 댓글에서 리뷰를 달아 두었습니다!
| private extension ChatBotListView { | ||
| func setupCollectionView() { | ||
| self.register(ChatBotMessageCell.self, forCellWithReuseIdentifier: "cell") | ||
| self.backgroundColor = .white | ||
| } |
There was a problem hiding this comment.
"cell" 이라는 이름보다 조금 더 명시적인 Identifier가 필요할 것 같아요.
| final class ChatBotMessageCell: UICollectionViewListCell { | ||
| private let userBubbleView = UserBubbleView() | ||
| private let systemBubbleView = SystemBubbleView() | ||
|
|
There was a problem hiding this comment.
userBubbleView와 systemBubbleView를 다른 셀로 분리하지 않으시고, 같은 셀에서 구현하신 이유가 궁금합니다!
| func setupMessageText(message: Message, type: String) { | ||
| switch type { | ||
| case "user": | ||
| self.configureUser(text: message.content) | ||
| case "assistant": | ||
| self.configureSystem(text: message.content) | ||
| default: | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Message의 있는 type 을 enum 으로 관리하게 되면 더 좋을 것 같네요.
Message 타입은 DTO Model 로 알고 있는데, 화면에 보이기 위한 데이터 타입을 만들어 보는 것도 좋을 것 같네요!
|
|
||
| final class ChatBotViewController: UIViewController { | ||
| private let chatInputTextView = ChatInputTextView() | ||
| private lazy var chatList = ChatBotListView() |
There was a problem hiding this comment.
ChatBotListView 를 따로 분리하신 이유가 궁금합니다!
| func setRightBubbleView(rect: CGRect) { | ||
| let bezierPath = UIBezierPath() | ||
| messageView.textColor = .white | ||
| let bottom = rect.height | ||
| let right = rect.width | ||
| bezierPath.move( | ||
| to: CGPoint(x: right - 22, y: bottom) | ||
| ) |
There was a problem hiding this comment.
Path 로 그린신 부분 👏🏻👏🏻👏🏻 정말 열심히 그리신 것 같습니다 🥹
나중에 Bubble image asset 이 있을 경우에는 image slicing 에 대해서 한번 찾아보시면, 더 쉽게 구현할 수도 있습니다😗
| lazy var inputTextField = UITextField().then { | ||
| $0.frame = CGRect(x: 0, y: 0, width: 0, height: 30) | ||
| $0.layer.cornerRadius = $0.layer.frame.height / 2 | ||
| $0.layer.borderWidth = 1 | ||
| $0.layer.borderColor = UIColor.lightGray.cgColor | ||
| $0.layer.masksToBounds = false | ||
| $0.placeholder = "메세지를 입력해주세요." | ||
| } |
AI 챗봇 앱 [STEP 3]
멤버 : @kimbs5899 @LeeSe0ngYe0n
리뷰어 : @cherrishRed
AI 챗봇 세번째 Step입니다!
정리하여 PR 드립니다. 감사합니다!
이번 스텝을 통해서 배운 점
RxSwiftSnapKitThen📁 구성
🤔 고민한 점
ViewModel 활용현재 뷰랑 연결되어서 데이터에 대한 세팅이나 설정을 모두 ViewController에서 하고 있습니다.
저희 로직중에 ViewModel로 빼거나 뺄수 있는 부분에 대해 말씀해주시면 감사하겠습니다!
Network 계층Network → NetworkProvider → ChatBotNetwork 이러한 계층을 갖고 있는데 이렇게 나누는게 맞는지 그리고 각각의 나눈 역할에 대한 장점을 잘 활용하고 있는지 의문입니다.
Trigger OnNext 활용Trigger를 통해 bind 하는 과정에서 onNext로 들어오는 데이터를 제대로 활용하지 못하는것 같아서 많이 아쉬웠습니다.
그냥 지금은 전에 델리게이트 처럼 통신하고 그 데이터를 갖다 쓰는것 뿐이지 제대로 Rx를 통해 OnNext에서 처리하는게 없어서 이러한 부분에 대한 방향성이 필요합니다!
🧐 궁금한 점
RxSwift, SnapKit, Then을 사용하면서현직에서 자주 사용 되고 이외에 추가적으로 더 공부하면 도움이 될거 같은 라이브러리를
추천해주시면 감사하겠습니다~