-
Notifications
You must be signed in to change notification settings - Fork 12
[김보성_Android] 9주차 과제 제출 #66
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
base: main
Are you sure you want to change the base?
[김보성_Android] 9주차 과제 제출 #66
Conversation
JaeYoung290
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
과제하시느라 수고 많으셨습니다!
| <TextView | ||
|
|
||
| <Button | ||
| android:id="@+id/OpenButton" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
xml의 naming convention은 lower snake case입니다. open_button과 같이 사용해주세요.
| /> | ||
|
|
||
| <TextView | ||
| android:id="@+id/text1" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
id는 어떤 뷰인지를 나타내는 네이밍이 좋습니다. tv_allow_permission 등 어떤 뷰인지 + 어떤 역할을 하는지를 명시해주시는 것을 추천드립니다.
| android:id="@+id/text1" | ||
| android:layout_width="wrap_content" | ||
| android:layout_height="wrap_content" | ||
| android:text="Allow permission to show files" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string resource 사용을 권장드립니다. 만약 일일이 추가하기 힘드시다면 ALT + Enter를 누를 경우 자동으로 추가해주는 옵션이 있으니 참고해 주세요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
추가로 몇가지 단축키를 추천드리자면
CTRL + ALT + L => 현재 파일의 코드 자동 포맷팅
CTRL + ALT + O => 불필요한 import문 삭제
CTRL + B 또는 ctrl 누르고 원하는 함수나 변수 클릭 => 해당 변수나 함수의 선언된 위치 표시 및 이동
CTRL + SHIFT + F => 전체 파일 또는 특정 위치에서 찾고 싶은 단어나 코드 검색
CTRL + SHIFT + R => 전체 파일 또는 특정 위치에서 원하는 코드나 문자열을 한번에 replace
CTRL + R => 위에 replace를 현재 파일에서만 사용
등이 있습니다!
|
|
||
| override fun onResume() { | ||
| super.onResume() | ||
| if (hasPermission()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
설정에서 돌아왔을 때 onResume에서 권한을 다시 확인하는 방식은 좋은 것 같습니다. 다만 onCreate에서도 권한을 허용했는지 확인하므로 2번을 검사하게 되는 비효율이 발생합니다.(hasPermission 자체는 가벼운 연산이지만 이후에 실행하는 showMusicList()는 목록을 update하는 것이므로 상황에 따라서 성능 저하가 발생할 수 있습니다) 따라서 이럴 때는 권한에 대한 상태를 변수로 저장하고 권한이 변경되었을 때만 권한을 update하도록 분기 처리 하는 방법도 고려해 보시면 좋을 것 같습니다.
| return ContextCompat.checkSelfPermission(this, permission) == PackageManager.PERMISSION_GRANTED | ||
| } | ||
|
|
||
| private fun requestPermission() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
권한 분기 처리를 잘 작성하신 것 같습니다!
|
|
||
| private fun showPermissionDialog() { | ||
| AlertDialog.Builder(this) | ||
| .setTitle(getString(R.string.permission_dialog_title)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
string resource 사용하신 부분이 좋은 것 같아요!
| } | ||
| } | ||
|
|
||
| musicAdapter.notifyDataSetChanged() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
notifyDataSetChanged는 항상 모든 리스트를 새로고침하므로 diffutil을 사용하는 방식도 추천드립니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹은 notifyItemInserted를 사용해도 됩니다
| @@ -1,14 +1,155 @@ | |||
| package com.example.bcsd_android_2025_1 | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
패키지명에 언더바는 사용하지 않는 것을 권장드립니다! lower camel case를 추천드려요
kongwoojin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
고생하셨습니다.
코멘트 확인해주세요
| testImplementation(libs.junit) | ||
| androidTestImplementation(libs.androidx.junit) | ||
| androidTestImplementation(libs.androidx.espresso.core) | ||
| androidTestImplementation(platform(libs.androidx.compose.bom)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
본 커리큘럼에서는 compose를 사용하지 않습니다.
| val cursor = contentResolver.query(uri, projection, null, null, null) | ||
| cursor?.use { | ||
| while (it.moveToNext()) { | ||
| val title = it.getString(0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
index를 하드코딩 하기보단, cursor.getColumnIndex(MediaStore.Audio.Media.TITLE)를 이용해 인덱스를 가져오는게 바람직해 보입니다.
| } | ||
| } | ||
|
|
||
| musicAdapter.notifyDataSetChanged() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
혹은 notifyItemInserted를 사용해도 됩니다
|
|
||
| override fun getItemCount(): Int = musicList.size | ||
|
|
||
| private fun formatDuration(ms: Long): String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Long.formatDuration()처럼 확장함수를 쓰는게 좋아보입니다
9주차