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주차 과제_step1 #27

Open
wants to merge 5 commits into
base: njiyeon
Choose a base branch
from

Conversation

nJiyeon
Copy link

@nJiyeon nJiyeon commented Jul 3, 2024

과제 수행을 하며 어려웠던 점

  • 데이터베이스를 다루는 것이 처음이라 낯설었다

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

  • 해당 방식으로 데이터를 로컬 데이터베이스에 생성하는 것이 맞나요..?
  • DB에 대해 공부하며 데이터의 일관성을 유지하기 위해 트랜잭션을 사용하는 것을 권장한다하여 db?.beginTransaction()을 사용했는데, 여기서 필요한 기능이 맞는지 궁금합니다.
  • 또한 트랜잭션과 관련하여 코드에 작성한 것처럼 사용하면 되는 게 맞을까요?
  • 데이터베이스를 형성할 때도 companion property를 활용하면 메모리 누수와 관련한 문제가 발생하는지 궁금합니다!

실행 화면

image

nJiyeon and others added 5 commits July 2, 2024 14:24
Copy link

@mkSpace mkSpace left a comment

Choose a reason for hiding this comment

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

처음이지만 훌륭하게 잘 작성해주셧습니다!

트랜잭션은 트랜잭션 내에서 에러가 발생할 경우 트랜잭션이 처음부터 실행되지 않은 시점으로 롤백이 됩니다. 롤백이 필요한 경우에 작성하면 되겠네요. 그럼 트랜잭션이 필요한 예시가 뭐가 있을까요? 계좌이체를 예로 들면 A가 B한테 입금을 한다고 했을 때 A의 계좌에서 잔액을 인출하고 B의 계좌에 입금을 해줘야 할 때 이 두 가지 Acting은 무조건 함께 가야 합니다. 하나만 실행되면 크나큰 손실이 있을 수 있습니다. 이런 경우 트랜잭션이 필요합니다. A의 계좌에서 잔액을 인출한 것만 실행하고 B의 계좌에 입금이 안되면 A의 돈만 증발해버릴 수 있을 테니 말이죠.

initial 데이터를 넣는데에 과연 트랜잭션이 필요할까? 라고 보면 사실 그렇진 않습니다. 없어도 괜찮은 데이터일 뿐더러 Cafe나 Pharmacy는 각각 넣어도 지장이 없기 때문입니다.그리고 코드는 훌륭하게 잘 작성해주셨습니다 ! 👍

마지막 질문인 데이터베이스를 형성할 때도 companion property를 활용하면 메모리 누수와 관련한 문제가 발생하는지 궁금합니다! 는 잘 이해가 가지 않는데 companion property에 어떤 변수를 넣으신다는건가요?

코드 읽어보고 리뷰 남깁니다. 의견 남겨주세요 ^^

noResultsTextView = findViewById(R.id.noResultsTextView)
resultsListView = findViewById(R.id.resultsListView)

dbHelper = PlaceDBHelper(this)
Copy link

Choose a reason for hiding this comment

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

dbHelper를 onDestroy 콜백에서 닫아주세요!


override fun onUpgrade(db: SQLiteDatabase?, oldVersion: Int, newVersion: Int) {
db?.execSQL(SQL_DROP_TABLE)
onCreate(db)
Copy link

Choose a reason for hiding this comment

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

on~~ 으로 붙은 함수들은 일반적으로 콜백 메서드로 사용됩니다. 내부 동작에 영향을 미치는 동작일 수 있기 때문에 Drop 후 create 하시려는거면 따로 함수를 빼서 작성하시는게 좋습니다.

Comment on lines +7 to +10
private const val TABLE_NAME = "places"
private const val COLUMN_CATEGORY = "category"
private const val COLUMN_TITLE = "title"
private const val COLUMN_LOCATION = "location"
Copy link

Choose a reason for hiding this comment

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

PlcaeContract 등으로 따로 클래스 만들어서 빼는게 좋겠네요

Copy link
Author

Choose a reason for hiding this comment

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

넵 step2에 위와 같이 수정하여 제출하였습니다 !

onCreate(db)
}

private fun insertInitialRecords(db: SQLiteDatabase?) {
Copy link

Choose a reason for hiding this comment

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

잘 사용하셨습니다! 👍

Comment on lines +64 to +65
private const val DB_VERSION = 1
private const val DB_NAME = "location.db"
Copy link

Choose a reason for hiding this comment

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

DB 관련 정보도 따로 빠지는게 좋겠네요. 여기서는 Place 관련 내용만 작성해서 괜찮지만 나중에 여러 테이블이 나오면 다른 테이블 관련 DBHelper에서 참조해야 한다면 의존성이 생기기에 따로 빼주세요.

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.

2 participants