-
Notifications
You must be signed in to change notification settings - Fork 51
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
[아크] 3, 4단계 오목 제출합니다. #51
Conversation
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.
선택 구현사항까지 아주 잘 구현해주셨네요 👍️
이미 코드가 깔끔해서 안드로이드와 관련된 몇 가지 사항들만 피드백했습니다.
확인 후 다시 요청해주세요!
override fun onCreate(db: SQLiteDatabase?) { | ||
db?.execSQL( | ||
"CREATE TABLE IF NOT EXISTS $TABLE_NAME_ROOM (" + | ||
"$TABLE_COLUMN_GAME_ID INTEGER PRIMARY KEY UNIQUE ON CONFLICT REPLACE," + | ||
"$TABLE_COLUMN_PLAYER_ID LONG UNIQUE," + | ||
"$TABLE_COLUMN_TITLE TEXT," + | ||
"$TABLE_COLUMN_STATUS INTEGER," + | ||
"$TABLE_COLUMN_TIME INTEGER" + | ||
");", | ||
) | ||
} | ||
|
||
override fun onUpgrade(db: SQLiteDatabase?, oldVersion: Int, newVersion: Int) { | ||
db?.execSQL("DROP TABLE IF EXISTS $TABLE_NAME_ROOM") | ||
onCreate(db) | ||
} |
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.
onCreate() 함수가 호출되지 않아서 테이블이 존재하지 않습니다. 언제 호출되는지 아래 글을 통해 확인해보고 수정해주세요.
https://stackoverflow.com/a/21881993/6686126
아마 개발 도중에 코드가 바뀌었는데 이미 존재하는 테이블이 있어서 놓치신 것 같아요.
val player = playerDb.getPlayer(nameText.text.toString()) ?: createPlayer(nameText) | ||
|
||
if (player == null) { | ||
Toast.makeText(this, "이름을 입력해주세요", Toast.LENGTH_SHORT).show() |
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을 하드코딩하는 대신 strings.xml
으로 옮겨주세요.
} | ||
|
||
private fun addListenerOnButton() { | ||
val button = findViewById<Button>(R.id.main_button) |
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.
findViewById()
함수는 View를 찾는 비용이 있어서 여러번 접근하게 되는 경우에는 onCreate()
함수에서 한 번만 찾고 변수에 캐시해두는게 좋습니다.
app/src/main/java/woowacourse/omok/dbHelper/OmokBoardDbModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/omok/dbHelper/OmokBoardDbModel.kt
Outdated
Show resolved
Hide resolved
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.
피드백 반영 확인했습니다.
마지막으로 사소한 코멘트 몇 가지 남겨두었습니다 🙂
지금까지 오목 미션 고생하셨어요!
@@ -12,17 +12,18 @@ import woowacourse.omok.dbHelper.OmokPlayerDbHelper | |||
import woowacourse.omok.roomList.RoomListActivity |
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.
refactor (Activity): 리뷰 반영
refactor (DbHelper): Db 생성 리뷰 반영
리뷰를 반영하여 수정할 때에도 커밋 메시지는 작업한 내용을 작성해주는게 좋습니다.
나중에 Git log를 열어보는 개발자는 리뷰 반영에는 큰 관심이 없고 실제로 어떤 작업이었는지가 궁금하기 때문이에요.
private val backButton by lazy { findViewById<Button>(R.id.back_button) } | ||
private val resetButton by lazy { findViewById<Button>(R.id.reset_button) } | ||
private val opposingPlayerImage by lazy { findViewById<ImageView>(R.id.opposing_player_image) } | ||
private val opposingPlayerName by lazy { findViewById<TextView>(R.id.opposing_player_name) } | ||
private val opposingPlayerScore by lazy { findViewById<TextView>(R.id.opposing_player_score) } | ||
private val playerImage by lazy { findViewById<ImageView>(R.id.player_image) } | ||
private val playerName by lazy { findViewById<TextView>(R.id.player_name) } | ||
private val playerScore by lazy { findViewById<TextView>(R.id.player_score) } | ||
private val omokBoard by lazy { findViewById<TableLayout>(R.id.board) } | ||
private val currentTurn by lazy { findViewById<TextView>(R.id.current_turn) } |
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.
이렇게 findViewById()
하나씩 작성해보니 조금 boilerplate code라고 느끼셨을텐데요.
나중에는 안드로이드에서 공식적으로 지원하는 View Binding을 활용해보세요!
안녕하세요! 리뷰어님!
첫 안드로이드 과제라서 학습하고 싶었던 것들이 많아서 이리저리 적용하다보니 제출이 늦게 되었습니다!
다음과 같이 구현들을 추가로 구현하였으며 아직은 기능들이 제한적으로 작동한다는 점은 양해 부탁드립니다!
도메인 로직 이외에 동작 방식은 다음과 같습니다!
우선 총 4가지의 액티비티가 존재합니다!
1. MainActivity
2. RoomListActivity
3. RoomActivity
4. RestartActivity
이렇게 안드로이드를 처음부터 작성하는 것이 처음이기에 맞는 구성으로 작성하였는 지 아직 많은 의문이 있지만 최대한 코드가 클린 할 수 있도록 작성을 하였습니다! 리뷰해주셔서 감사합니다!