send message#5
Conversation
yooxdb
left a comment
yooxdb
left a comment
There was a problem hiding this comment.
실제 디바이스를 이용한 테스트가 불가능하다보니 기울기/가속 감지 로직에 대해서는 깊게 리뷰를 남기기 어렵네요. 그 외의 부분들 위주로 리뷰 남깁니다. 코멘트 달지 않은 부분에도 전반적으로 Swift 스타일에 맞지 않는 코드가 많이 보여서, 한번 쭉 보면서 수정해주시면 좋을 것 같아요 수고하셨습니다~
| if (!directionFiltered.isEmpty){ | ||
| let directionSorted = directionFiltered.sorted(by: { | ||
| norm_one($0.direction!) < norm_one($1.direction!) | ||
| }) | ||
| return directionSorted.first!.discoveryToken | ||
| }else{ | ||
| let distanceSorted = nearbyObjects.sorted { $0.distance ?? .zero < $1.distance ?? .zero } | ||
| return distanceSorted[0].discoveryToken | ||
| } |
There was a problem hiding this comment.
사소하지만..Swift 스타일로 수정해주시면 좋을 것 같아요
| //MARK: - 뷰 관련 값 | ||
| @Published var emoji : String = "" | ||
| @Published var content : String = "" |
There was a problem hiding this comment.
여기 emoji랑 content를 뷰모델로 뺀 이유가 있나요? 뷰모델에서 뷰로 publish할 필요가 있을지 궁금하네요
There was a problem hiding this comment.
메세지를 보내는 로직이 뷰쪽에 있으면 mvvm 구조랑 맞지 앚는 것 같아서 뷰모델으로 빼놓았습니다. 뷰에서는 뷰모델의 viewstate 값에 따라서 중간 레이블 값만 (타이머, 전송 성공, 전송 실패, 모션 감지 실패 등) 바꿀 수 있도록 했고, 뷰에서 모션 감지나 보내는 로직이 들어가 있으면 복잡할 것 같아서요. 어떻게 생각하시나요?
There was a problem hiding this comment.
네 메시지를 보내는 로직이 뷰모델이 있는건 맞는데, emoji랑 content 자체는 뷰모델의 지시에 따라 바뀌어야하는 값이 아니므로 뷰가 독립적으로 들고 있다가 뷰모델쪽으로 그때그때 넘겨주는 방식이 맞아보여요
| //MARK: - 메세지 전송 관련 값 | ||
| let motionManager = CMMotionManager() | ||
| @Published var sendTimer: Timer? | ||
| @Published var currentState: viewState = .none | ||
| @Published var isSending: Bool = false | ||
| @Published var isAccelerating: Bool = false | ||
| @Published var accelerationRate: Double = 0.0 | ||
| @Published var counter: Int = 0 | ||
| @Published var isReadyForSending: Bool = false | ||
| var isDetected: Bool = false |
There was a problem hiding this comment.
이 변수들 모두 view 내부에서만 사용되고 뷰모델을 통한 세션과의 연결은 필요하지 않으므로 특별한 이유가 없다면 뷰에 있는 것이 맞아보이는데 혹시 어떻게 생각하시나요
| .onChange(of: viewModel.content) { text in | ||
| if text.last == "\n" { | ||
| viewModel.content = String(text.dropLast()) | ||
| UIApplication.shared.sendAction(#selector(UIResponder.resignFirstResponder), to:nil, from:nil, for:nil) | ||
| } | ||
| } |
There was a problem hiding this comment.
이 부분은 입력값의 마지막이 개행 문자인지 검사하는 것보다는 .onSubmit 등을 이용하는 것이 어떨까요? 그리고 @FocusState를 이용 중이므로 UIApplication ~ 한 줄을 위해 UIKit을 import하는 것보다는 SwiftUI스럽게 isTextFieldFocused = false로 수정하면 어떨지 제안 드립니다
There was a problem hiding this comment.
이 부분은 지호님께서 수정하신 부분인것 같은데 깃에서 제가 추가한걸로 인식한 것 같네요!
2 participants
No description provided.