Skip to content
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

[Feature] 사진 오브젝트 전송 로직 수정, Text 오브젝트 수정 기능 추가 #117

Merged
merged 7 commits into from
Nov 26, 2024

Conversation

taipaise
Copy link
Collaborator

🌁 Background

  • 오브젝트 수정 기능 완성

👩‍💻 Contents

  • TextObject 수정 기능 추가
  • 사진 오브젝트 전송 시, 사진 데이터도 함께 전송

✅ Testing

  • 시뮬레이터 빌드 후 테스트
  • 시간 상의 이유로 텍스트 수정 테스트코드는 작성하지 못했습니다.

📝 Review Note

  • TextObject를 적절한 방법으로 수정해야하는데, 시간 상의 이유로 간단한 방법으로 해결하고자 했습니다. TextObjectView를 생성할 때 UITextViewDelegate를 채택한 VC를 파라미터로 넘겨주는 방법인데요, 이로서 TextObjectView의 TextView에서 일어나는 action들을 VC가 감지하고 처리하게 됩니다. 점점 키메라가 되어가는거 같은데,, 개선 사항이 하나 둘 씩 늘어나고 있군요
  • 사진 오브젝트를 전송할 때, 파일 시스템에 저장한 사진은 안넘기고 저장한 사진의 url만 넘기고 있다는 것을 알았습니다. 이에 사진 오브젝트는 수정/추가 시 따로 사진 데이터를 전송하는 코드를 작성했습니다. 다만 문제점이 있는데요, 사진의 정보가 수정될 때, 원본 사진 데이터는 전송할 필요가 없지만 항상 같이 전송한다는 문제가 있습니다. 이 부분은 개선 포인트 0순위라고 생각합니다.

@taipaise taipaise added this to the 화이트보드 수정 milestone Nov 26, 2024
@taipaise taipaise self-assigned this Nov 26, 2024
Copy link
Collaborator

@choijungp choijungp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

딩푸루부 ............ 점점 더 오추팀의 로직을 이해하기 어려워지고 있어유 ..
분발하겠습니다 😇😇😇😇😇😇😇😇😇😇😇

@@ -12,4 +12,5 @@ public enum AirplaINDataType: String, Codable {
case drawing
case game
case chat
case jpg
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

photo와 jpg의 차이가 무엇인가요 ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

photo는 whiteboardObject이고, jpg는 실제 사진 data에 해당합니다
실제 사진 데이터 case에 어떤 이름을 붙이는게 적절할까요!! 네이밍 도와주세요,,

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5f9ff2a 반영 했습니다!

Comment on lines 10 to 20
public protocol WhiteboardObjectSetInterface {
func contains(object: WhiteboardObject) async -> Bool

func insert(object: WhiteboardObject) async

func remove(object: WhiteboardObject) async

func update(object: WhiteboardObject) async

func fetchObjectByID(id: UUID) async -> WhiteboardObject?
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

얘네 !!! 인터페이스라 ~ 문서주석 필요합니뎅 ~~
그른데 WhiteboardObjectSetInterface 요 녀석은 정확히 무순 용도인가요 ?

ManageWhiteboardObjectUseCase에서 쓰이던데 WhiteboardObjectSet 관리를 위한 인터페이스지만,
UseCase, Repository 추상체가 아니라 그냥 Interface 폴더에 넣으신건가요 ??

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 맞습니다!! 폴더링이 애매해서 일단은 Interface 폴더에 넣었습니다.
WhiteboardObject들을 안전하게 꺼내 쓸 수 있는 WhiteboardObjectSet을 추상화한 프로토콜입니다!
WhiteboardObject 수정이 ManageWhiteboardUseCase와 AddTextUseCase 두 군데에서 사용되고 있기 때문에 추상화하였습니다.
추가로 AddTextUseCase의 경우 이름을 바꾸는게 적절해보이네요~

Copy link
Collaborator Author

@taipaise taipaise Nov 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 이미 AddTextUseCase가 아니라 지금도 TextObjectUseCase였네요!! 현상유지하는걸로~

주석은 5f9ff2a 요기서 반영 했습니다!

Comment on lines +28 to +29
whiteboardRepository: WhiteboardObjectRepositoryInterface,
whiteboardObjectSet: WhiteboardObjectSetInterface
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

쏘 복잡 .... ....

whiteboardRepository에서는 NearbyNetworkInterface에 데이터를 전송하는 역할,
whiteboardObjectSet에서는 말 그대로 whiteboardObject Set의 데이터를 관리하는 역할 !! 로 나뉘는 것인가요 ?!?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 맞습니다!!

@taipaise
Copy link
Collaborator Author

딩푸루부 ............ 점점 더 오추팀의 로직을 이해하기 어려워지고 있어유 .. 분발하겠습니다 😇😇😇😇😇😇😇😇😇😇😇

이건 설계 구현의 문제가 아닐까,, 개선점이 쌓여가고 있습니다!! 타임어택을 눈 앞에 두니 포기하는 부분이 많아지는데요!! 같이 리팩..해주실거죠?

Copy link
Member

@eemdeeks eemdeeks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

고생하셨습니다ㅜㅜ

어쩌다보니 오브젝트 추가 및 수정의 90프로를 혼자서 담당하시게 된것같아 죄송합니다ㅜㅜ

하지만 딩동이 맡은 덕분에 빠르게 해결.. 구욷

func save(
dataInfo: DataInformationDTO,
data: Data?,
fileType: String?) -> URL?
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fileType을 언젠가는 enum으로 구분해 주면 좋을 것 같네요..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

알고보니~~!! 필요 없어서 다음 PR에서 삭제 될 예정입니다!!
추후 추가 된다면 enum으로 제한하는거 완전 동의합니다!

Comment on lines 36 to 40
async let sendJPEGTask: () = sendJPEG(
photoObject: photoObject,
type: .jpg,
isDeleted: isDeleted)
async let sendPhotoObjectTask: () = send(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

async let vs let await
....

학습해봐야겠군요..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TaskGroup에서 자식 task를 생성해서 처리했던 것처럼, async let을 이용하며 parallel하게 여러 비동기 작업을 진행할 수 있습니다!
구조적 동시성을 키워드로 찾아보면 좋을 것 같습니다~!

Comment on lines +103 to +105
guard let savedURL = filePersistence.save(dataInfo: info, data: receiveData, fileType: ".jpg") else { return }

if !info.isDeleted {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

추후에 delete됐다면 로컬 파일을 삭제한다거나,

화이트보드를 나온다면 파일들을 삭제한다는 정책을 정해보면 좋을 것 같네요...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

동의 합니다!!! 가능하면 내일 작업해보겠습니다!

@@ -43,4 +43,8 @@ public class TextObject: WhiteboardObject {
try container.encode(text, forKey: .text)
try super.encode(to: encoder)
}

func updateText(text: String) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

스타일적인 부분이라 가볍게 생각해주세요!
저는 func update(text: String) 이런 형식으로 주로 사용합니다!!
텍스트를 업데이트하다 이런 느낌으로!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

파라미터에서 text임을 알려주고 있으니까 좋은 방법이라고 생각합니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5f9ff2a 반영 했습니다!

@taipaise taipaise merged commit f9d2115 into develop Nov 26, 2024
2 checks passed
@taipaise taipaise deleted the feature/manageObjects branch November 27, 2024 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants