[Feature] 화이트 보드 오브젝트/도구 관리 유즈케이스 추가 #86
Conversation
| // 존재하지 않는 화이트보드 오브젝트 삭제 실패하는지 테스트 | ||
| func testRemoveNonExistentObject() { | ||
| // 준비 | ||
| let object = WhiteboardObject(id: UUID(), position: .zero, size: CGSize(width: 100, height: 100)) | ||
| var receivedObject: WhiteboardObject? | ||
|
|
||
| useCase.removedWhiteboardObject | ||
| .sink { receivedObject = $0 } | ||
| .store(in: &cancellables) | ||
|
|
||
| // 실행 | ||
| let result = useCase.removeObject(whiteboardObject: object) | ||
|
|
||
| // 검증 | ||
| XCTAssertFalse(result) | ||
| XCTAssertNil(receivedObject) | ||
| XCTAssertEqual(useCase.fetchObjects().count, 0) |
There was a problem hiding this comment.
테스트 잘 봤습니다 !!
궁금한 것이 있는데요 왜 존재하지 않는 화이트보드 오브젝트 삭제는 테스트했는데
존재하지 않는 화이트보드 오브젝트 업데이트 테스트는 없나용 ??
There was a problem hiding this comment.
테케 생각을 못했습니다..! 날카로운 조이~
There was a problem hiding this comment.
3651910 에서 반영했습니다!
| /// 현재 존재하는 모든 화이트보드 객체들을 가져옵니다. | ||
| /// - Returns: 화이트보드 위에 존재하는 화이트보드 객체들 | ||
| func fetchObjects() -> [WhiteboardObject] |
There was a problem hiding this comment.
구현체와 주석을 같이 본 바로는 본인이 갖고 있는 화이트보드 오브젝트들을 가져오는 메소드로 보이는데요,
혹시 언제 어떻게 사용될지 알 수 있을까요?
어떻게 사용될지, 필요한지 예상이 되지 않아서 코멘트 남깁니다 :)
There was a problem hiding this comment.
앗 이 부분 사용될 일이 없을것 같습니다!! 삭제하도록 하겠습니다!
There was a problem hiding this comment.
692d764 에서 반영하였습니다.
| } | ||
|
|
||
| public func updateObject(whiteboardObject: WhiteboardObject) -> Bool { | ||
| guard let index = whiteboardObjects.firstIndex(where: { $0 == whiteboardObject }) else { return false } |
There was a problem hiding this comment.
업데이트 한 whiteboardObject를 비교해야하기 때문에,
WhiteboardObject 의 equatable메소드에서 id만 비교한 부분이 인상 깊어요!
| _ = useCase.addObject(whiteboardObject: object1) | ||
| _ = useCase.addObject(whiteboardObject: object2) | ||
| _ = useCase.addObject(whiteboardObject: object3) |
There was a problem hiding this comment.
반환값 사용을 하지 않을 때,
let _ 가 아닌 _ 만 사용해도 되는군요..?
There was a problem hiding this comment.
넵넵 맞습니다!! 요 부분도 고민 중인데요, discardableResult를 고려중입니다!!
| } | ||
|
|
||
| // 화이트보드 위에 존재하는 오브젝트들 가져오기 성공 테스트 | ||
| func testFetchObjects() { |
There was a problem hiding this comment.
오브젝트 가져오는 fetchObject메소드의 테스트인데, 테스트 내부에서 addObject를 사용해야 하는 것이 조금 걸리네요..!
테스트코드를 많이 작성 해보지는 않아 더 좋은 방법이 떠오르지는 않는게 아쉽네요ㅜㅜ
addObject에 대한 테스트도 진행 해주니 괜찮겠죠??
There was a problem hiding this comment.
헉 저도 같은 고민을 했는데, 더 나은 해결 방법을 생각하지 못해서 일단 위와 같이 작성했습니다!
이 부분은 팀 내에서 한번 더 이야기 나눠보면 좋을 것 같습니다.
There was a problem hiding this comment.
좋아요!
테스트 코드 작성 해 주시는 거 너무 좋습니다!!
| // | ||
|
|
||
| import Combine | ||
| import Domain |
There was a problem hiding this comment.
Domain 내의 있는 테스트인데도, Domain을 import 해야되는 군요..?
|
|
||
| public protocol ManageWhiteboardToolUseCaseInterface { | ||
| /// 화이트보드 도구 선택/선택 해제 시 이벤트를 방출합니다. | ||
| var currentToolPublisher: AnyPublisher<WhiteboardTool?, Never> { get } |
There was a problem hiding this comment.
ManageWhiteboardObjectUseCaseInterface 와 공통으로 코멘트 남깁니다!
목요일에 나눴던 얘기와 이어서 진행이 될 것 같은데요,
메소드의 반환 값으로 Publisher를 반환하는 방법이 생각나는데, 해당 방법 말고 따로 인터페이스에 추가해주신 이유가 있을까요??
There was a problem hiding this comment.
하나의 퍼블리셔를 단발성으로 사용하지 않고 지속적으로 사용하게 되지 않을까 하여 위와 같이 구현했습니다.
또한 메서드의 반환 값으로 Publisher를 반환하게 되면, 해당 퍼블리셔를 매번 새로 구독해야할 것 같습니다!
이렇게 되면 Cancellable에 실제로는 사용하지 않는 구독이 점점 많이 쌓이게 될 것이라 생각합니다. (Tool을 바꿀 때마다 새로 구독해야 함) 이는 곧 메모리 누수로 이어지지 않을까 합니다!! 혹시라도 제가 잘못 알고 있는게 있다면 알려주시면 감사하겠습니다!!!
There was a problem hiding this comment.
설명 감사합니다!!
많은 고민이 담겨있는 코드였군요!!
역시..구욷
좋은 방법이라고 생각합니다 :)
| whiteboardObjects[index] = whiteboardObject | ||
| updatedWhiteboardSubject.send(whiteboardObject) | ||
| return true | ||
| } |
There was a problem hiding this comment.
updateObject에는 이후에 추가적인 로직이 작성된다고 이해해도 될까요?
Result type을 활용하는 것 대신 Bool을 반환하고, subject를 통해 send해주는 방식을 선택한 이유도 궁금합니다!
There was a problem hiding this comment.
넵 맞습니다! 아직 해당 에픽까지 진행하지 않아 뼈대만 구현한 상태입니다.
Result 타입이 아닌 Bool을 반환한 이유는
1. 아직 error 케이스를 정의하지 않았고 (실패 정책이 없어서)
2. 성공 실패만 따지면 되지 않을까?
하여 Result 타입 대신 Bool을 return 하도록 했습니다.
subject를 send하는 방식은 유즈케이스를 사용하는 쪽에게 '업데이트 된 객체' 라는 이벤트를 발행하기 위함입니다!
🌁 Background
👩💻 Contents
✅ Testing
📝 Review Note
@eemdeeks @choijungp @ekrud99