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

음악 검색 및 재생 라이브러리 연동 #155

Merged
merged 13 commits into from
Dec 6, 2022

Conversation

radiantchoi
Copy link
Collaborator

12/1~12/5 #141 #142 #147 #148 #149

작업한 내용

음악 검색을 위한 샤잠킷을 연동했습니다.

  • 일단 작성 뷰 컨트롤러에 붙어 있습니다.

음악 재생을 위한 뮤직킷을 연동했습니다.

  • 마찬가지로 일단 일기 상세 뷰 컨트롤러에 붙여 두었습니다.

고민한 점 및 어려웠던 점

샤잠킷의 결과에 따라 처리하는 것에 대해 고민했습니다.

  • 검색 성공, 검색 실패뿐만 아니라 검색 중단에 대해서도 고려해야 했기 때문에, 이 경우 어떤 이벤트를 보내야 할지 고민했습니다.
  • 구조에 대해 더 생각해보고 리팩토링할 때 해결할 예정입니다. 많은 조언 부탁드립니다.

피드백 결과로, 음원을 찾았을 때 앨범 아트를 어디에 표시해줄지 고민했습니다.

  • 지금 생각으로는 검색 버튼의 이미지를 바꾸는 방식으로 하면 좋지 않을까 생각합니다.

Comment on lines 133 to 135
private func parseBodyFromJWTData(_ jwtData: [Data]) -> Data {
if jwtData.count > 1 {
return jwtData[1]
Copy link
Member

Choose a reason for hiding this comment

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

요 부분 더이상 사용하지 않는 부분이니 삭제해도 좋을 것 같습니다

Copy link
Member

Choose a reason for hiding this comment

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

수정된 코드에선 segments[1] 만을 디코딩하는데 사용하네요? 저랑 태호님이 만나서 테스트했을 땐 두번째 요소만 디코딩하면 결과가 때떄로 이상하게 나와 count에 따른 케이스 분류를 해줬는데, 변경된 코드가 원활히 동작하는지 계속 지켜봐야 할 것 같습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

일단 쓰이지 않는 부분은 삭제해 주었습니다! 말씀해주신 부분은 계속 모니터링이 필요할 것 같네요.

self.result.onNext(.failure(.recordDenied))
}
}
@unknown default:
Copy link
Member

Choose a reason for hiding this comment

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

unknown default는 처음 접해보네요. 이런 기능이 있었군요!

Copy link
Collaborator Author

@radiantchoi radiantchoi Dec 6, 2022

Choose a reason for hiding this comment

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

그냥 default랑 동작은 같습니다!! 알려지지 않은 경우에 대비하는 default라고 합니다. 애플이 만든 프레임워크다 보니까 패치하고 나면 어떻게 기능이 추가될지 모르는 거고, 그런 경우에 unknown을 사용해 주면 나중에 switch문이 exhaustive하지 않을 때 노란 줄 에러로 알려 준다고 하네요.

Copy link
Member

@LEEYOONJONG LEEYOONJONG left a comment

Choose a reason for hiding this comment

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

고생하셨습니다!! parseBodyFromJWTData 함수만 삭제 부탁드리겟습니다!!

Copy link
Collaborator

@rudah7 rudah7 left a comment

Choose a reason for hiding this comment

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

해야할 일들이 많군요 ㅠㅠ
전반적으로 코드 이해를 해봐야할 것 같습니다..!

수고하셨습니다!

}

// 뷰 컨트롤러를 나갈 때 큐를 비워 준다.
// 뮤직세션, 샤잠세션은 싱글턴 인스턴스로 만들어 주는 것이 좋겠다.
Copy link
Collaborator

Choose a reason for hiding this comment

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

TODO(?) 작성이 잘 되어있군요! 좋습니다~ ㅎㅎ

@@ -54,3 +54,9 @@ struct DiaryDetail: Encodable {
)
}
}

#if DEBUG
Copy link
Collaborator

Choose a reason for hiding this comment

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

지난번에 고민했던 것이 반영되었군요! 좋습니다 :)

Copy link
Member

@LEEYOONJONG LEEYOONJONG left a comment

Choose a reason for hiding this comment

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

고생하셨씁니다!!!

@radiantchoi radiantchoi merged commit aaa1d24 into develop Dec 6, 2022
@radiantchoi radiantchoi deleted the feature/search-and-play-music branch December 6, 2022 04:49
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.

3 participants