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

부산대 Android_김현민 2주차 과제 수정 #72

Open
wants to merge 16 commits into
base: khyeonm
Choose a base branch
from

Conversation

khyeonm
Copy link

@khyeonm khyeonm commented Jul 9, 2024

어려웠던 점

  • MVVM에 대해 이해가 어려워서 이번 주차에서는 완벽하게 적용하지 못한 것 같습니다.
  • 시간 관계상 step2 코드를 다 짜두고 commit을 남기는 과정으로 진행해서 commit내 코드 변경사항이 매끄럽게 적용되지 않은 것 같습니다.

중점적으로 리뷰해주셨으면 하는 부분

  • RecyclerView 구현이 전체적으로 잘 되었는지 궁금합니다.
  • 메소드 분리가 적절하게 잘 되었는지 궁금합니다.

궁금한 점

  • 과제 참고 영상에는 저장된 장소로 기존에 있던 장소가 선택되었을 경우 switch되는 효과가 적용된 것 같은데 해당 효과는 어떻게 적용된 것인지 궁금합니다.
  • 메소드 분리가 적절하게 잘 되었는지 궁금합니다.

재제출

  • 충돌 해결 과정에서 코드에 오류가 있었던 것 같습니다..! 기능 동작 영상도 함께 첨부합니다!
KakaoTalk_20240709_175235343.mp4

Copy link

@LeeOhHyung LeeOhHyung left a comment

Choose a reason for hiding this comment

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

@khyeonm
LGTM~ 과제 수정 PR에서는 마지막 커밋위주로 확인했습니다. 고생많으셨습니다.

Comment on lines 36 to 40
val db = this.writableDatabase
private fun insertPharData(db: SQLiteDatabase?) {
val type = "약국"
val name = "약국"
val address = "서울 강남구 대치동"

for (i in 1..30) {
val name = "$name$i"
val address = "$address $i"
db.execSQL("INSERT INTO $TABLE_PLACE ($COLUMN_TYPE, $COLUMN_NAME, $COLUMN_ADDRESS) VALUES ('$type', '$name', '$address');")
val nameWithIndex = "$name$i"
val addressWithIndex = "$address $i"
db?.execSQL("INSERT INTO $TABLE_PLACE ($COLUMN_TYPE, $COLUMN_NAME, $COLUMN_ADDRESS) VALUES ('$type', '$nameWithIndex', '$addressWithIndex');")

Choose a reason for hiding this comment

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

db가 null 인지 체크를 미리해서, early return 시키거나 null일때는 이 메소드가 호출안되도록 하는게 좋을것 같습니다. 왜냐면, db가 null 일때도 loop를 30회 수행하고 있을것이라 무의미한 연산이 수행될것 같네요.

Choose a reason for hiding this comment

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

아래 insertCafeData 메소드도 동일합니다.

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.

2 participants