Skip to content

feat: thumbnail 이미지 다운로드 개선#274

Merged
anyukyung merged 7 commits intoiOS/devfrom
iOS/feat#258
Dec 10, 2023
Merged

feat: thumbnail 이미지 다운로드 개선#274
anyukyung merged 7 commits intoiOS/devfrom
iOS/feat#258

Conversation

@anyukyung
Copy link
Member

🧑‍🚀 PR 요약

해당 pr에서 작업한 내역을 적어주세요.

  • 기존 ThumbnailLoadedPost 모델을 삭제하고, 공통 Post모델에 thumbnailImageData를 추가했습니다.

  • 주된 작업은 기존 썸네일 다운로드 작업에 TaksGroup을 사용해서 병렬적으로 다운로드 처리를 했다는 점인데
    이미 다른 분들도 적용해보시고, 아시는 부분이라고 생각해 자세한 설명은 안하겠습니다 !

  • 적용하면서 배운 것은 🔎 미루고 미루던 async await 공부 (feat. 이미지 다운로드 병렬 처리)에 간단하게 정리해두었습니다.

  • 다만 ThumbnailImageData 다운로드에 실패했을 때 어떻게 처리할지 논의해보면 좋을 것 같아요.
    저는 썸네일 다운로드에 실패하면 data를 nil로 넣고 영상은 재생할 수 있도록 처리해두었는데, 아예 실패한 경우 영상도 안보여주도록 처리할 수도 있을 것 같아요.

  • 또한 지금 Post모델에 thumbnailImageData를 변수로 추가해서 아래와 같은 형식으로 사용했는데요.

var thumbnailLoadedPost = post.toDomain()
thumbnailLoadedPost.thumbnailImageData = thumbnailImageData
return thumbnailLoadedPost
  • 더 좋은 방법이 있을지 고민입니다,,

📌 변경 사항

  • 기존 ThumbnailLoadedPost 모델을 삭제
  • 공통 Post모델에 thumbnailImageData 변수 추가
📸 ScreenShot

작동, 구현화면

  • 원래는 썸네일 로드 과정에서 Delay가 있었는데, 눈에 띄게 개선됐습니당
RPReplay_Final1702205422.MP4

Linked Issue

close #

Copy link
Collaborator

@chopmozzi chopmozzi left a comment

Choose a reason for hiding this comment

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

확인했습니다!

Comment on lines +45 to +59
private func fetchThumbnailImageData(of posts: [PostDTO]) async -> [Post] {
await withTaskGroup(of: Post.self, returning: [Post].self) { group in
for post in posts {
group.addTask {
let thumbnailImageData = try? await self.provider.request(url: post.board.videoThumbnailURL)
var thumbnailLoadedPost = post.toDomain()
thumbnailLoadedPost.thumbnailImageData = thumbnailImageData
return thumbnailLoadedPost
}
}
var thumbnailLoadedPosts: [Post] = []
for await thumbnailLoadedPost in group {
thumbnailLoadedPosts.append(thumbnailLoadedPost)
}
return thumbnailLoadedPosts
Copy link
Collaborator

Choose a reason for hiding this comment

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

공통적인 Worker로 빼서 만드는게 아니라 각자 Worker에서 쓰도록 하는 거죠?

Copy link
Member Author

Choose a reason for hiding this comment

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

어느정도 모델 어떻게할지 정하면 공통 워커로 뺄 수 있지 않을까 싶습니다 ?
지금 Home이랑 Map에서 쓰는 로직이 아예 똑같아서리..

Copy link
Collaborator

Choose a reason for hiding this comment

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

중복되면 UserWorker처럼 따로 빼두는 것도 좋긴 하네요 모델 어떻게 할 지만 정하면 되겠군요.

let member: Member
let board: Board
let tag: [String]
var thumbnailImageData: Data?
Copy link
Collaborator

@loinsir loinsir Dec 10, 2023

Choose a reason for hiding this comment

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

다른 방법 고민해봤는데, 이 방법이 기존 도메인 모델을 그대로 사용할 수 있으면서, 우리가 원하는 동작을 할 수 있어서 좋은 것 같습니다.
그리고 어차피 다른 모델을 정의한다고 해도, struct여서 값 복사를 필연적으로 이뤄질 수 밖에 없는데,
이 방법은 그런 값 복사에서의 실수도 방지할 수 있겠네요
이 방법으로 적용하는 거 전 찬성하겠습니다👍

Copy link
Collaborator

@loinsir loinsir left a comment

Choose a reason for hiding this comment

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

나머지 부분은 특이사항 없어서 approve 하겠습니다~
이거 머지 해주시면 제 pr도 변경하신 모델 맞춰서 수정할게요

@loinsir
Copy link
Collaborator

loinsir commented Dec 10, 2023

다만 ThumbnailImageData 다운로드에 실패했을 때 어떻게 처리할지 논의해보면 좋을 것 같아요.
저는 썸네일 다운로드에 실패하면 data를 nil로 넣고 영상은 재생할 수 있도록 처리해두었는데, 아예 실패한 경우 영상도 안보여주도록 처리할 수도 있을 것 같아요.

제가 실패한 경우에 홈쪽에 영상도 안보이도록 우선 설정해두긴 했는데요, 만약에 썸네일 다운로드 실패면 uiimageview의 backgroundColor 설정을 어둡게 해서라도 띄울까 하는데 어떨까요? 스켈레톤 뷰 처럼요

@anyukyung
Copy link
Member Author

다만 ThumbnailImageData 다운로드에 실패했을 때 어떻게 처리할지 논의해보면 좋을 것 같아요.
저는 썸네일 다운로드에 실패하면 data를 nil로 넣고 영상은 재생할 수 있도록 처리해두었는데, 아예 실패한 경우 영상도 안보여주도록 처리할 수도 있을 것 같아요.

제가 실패한 경우에 홈쪽에 영상도 안보이도록 우선 설정해두긴 했는데요, 만약에 썸네일 다운로드 실패면 uiimageview의 backgroundColor 설정을 어둡게 해서라도 띄울까 하는데 어떨까요? 스켈레톤 뷰 처럼요

오오 지금 홈, 지도에서 쓰는 셀은 영상위에 썸네일 올라가있는 형태이죠 ?? 그것도 괜찮은 방법일 것 같습니다 ㅎㅎ
저는 지도 핀에서 썸네일 없을 경우 empty 이미지 띄워주는걸로 처리하려 했는데, 인환님 말씀하신 방법이랑 비슷할 것 같습니다
empty 이미지 하나 간단하게 만들어서 공통으로 써도 괜찮을듯ㅎㅎ

@anyukyung anyukyung linked an issue Dec 10, 2023 that may be closed by this pull request
1 task
@anyukyung anyukyung merged commit 0862409 into iOS/dev Dec 10, 2023
@anyukyung anyukyung deleted the iOS/feat#258 branch January 10, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: thumbnail 이미지 다운로드 개선

3 participants