-
Notifications
You must be signed in to change notification settings - Fork 33
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_이창욱_1주차_과제 #5
base: ichanguk
Are you sure you want to change the base?
Conversation
- repository 설명 추가 - 구현할 feature list 추가
이름 EditText 또는 전화번호 EditText가 비어있을 때 토스트 메시지 출력 기능 추가
전화번호 입력을 숫자로 제한하도록 함
이름과 전화번호 입력 외에 나머지 EditText UI 추가
- 더보기를 위한 레이아웃 추가 - 더보기를 누를 때 나오는 EditText 뷰들은 visibility의 기본값을 gone으로 설정
더보기를 누르면 나머지 입력폼들이 보여지는 기능 추가
gender text와 gender를 선택하는 radiogroup을 하나의 layout으로 묶음
저장 버튼을 누르면 저장 완료 토스트 메시지 출력
취소 버튼을 누르면 취소 토스트 메시지 출력
성별 라디오 버튼을 선택하면 gender 변수에 성별이 저장되도록 함
기능 리스트에 생일 입력 기능 추가
생일 입력 폼을 클릭하면 캘린더에서 생일을 고를 수 있도록 함
취소, 저장 버튼을 제외한 나머지 레이아웃들을 ScrollView 안에 넣어 스크롤할 수 있도록 함
2단계를 위한 기능 리스트 추가
연락처 등록 화면 UI 디자인
+ 버튼을 누르면 연락처 작성 액티비티로 넘어가도록 구현
- 연락처 목록을 표시하기 위한 리사이클러뷰 추가 - 리사이클러뷰를 위한 아이템 UI 추가
기능리스트의 3번 내용(목록 스크롤)을 2번 내용(목록에 추가)에 추가
연락처를 저장하면 목록에 추가되도록 구현
화면 전환 시 연락처 목록이 초기화되지 않도록 함수 추가
연락처 목록의 연락처를 클릭하면 상세화면이 보여지도록 함
뒤로가기 외에 취소 버튼을 눌렀을 때도 확인 팝업이 나오도록 기능 변경
뒤로가기/ 취소 버튼 클릭 시 확인 팝업이 나오도록 구현
./gradlew ktlintCheck와 ./gradlew ktlintFormat를 사용해 리팩토링
contents 이미지 추가
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.
안녕하세요 창욱님! 앞으로 함께할 강대규 멘토입니다.
앞으로 6주동안 잘 부탁드려요 :)
커밋 단위가 적절한지 궁금합니다.
커밋단위 매우 적절하게 잘 작성해주셨어요. 걱정하시는 것보다 더욱 잘 작성해주셨어요.
data class를 Intent로 전달하는 과정이 효율적으로 잘 구현이 됐는지 봐주시면 좋을 것 같습니다.
type casting 도 잘 신경써주셨고, parcelable 로 정의한 부분도 매우 인상깊었습니다. 매우 좋습니다 👍
함수들이 적절하게 분리되어 사용되고 있는지 봐주시면 좋을 것 같습니다.
평소에 습관이신지는 모르겠지만, 매우 잘하고 계시네요! 더할나위 없이 깔끔한데요!?
@@ -1,6 +1,8 @@ | |||
plugins { | |||
id("com.android.application") | |||
id("org.jetbrains.kotlin.android") | |||
id("kotlin-parcelize") | |||
id("org.jlleitschuh.gradle.ktlint") |
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.
오호... ktlint 설정까지 해주실 예정인걸까요?
@Parcelize | ||
data class Contact( | ||
val name: String, | ||
val phoneNum: String, | ||
val email: String, | ||
val birthday: String, | ||
val gender: Int, | ||
val memo: String, | ||
) : Parcelable |
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.
Parcelize 아주 좋습니다! 👍
private fun initViews() { | ||
detailNameTextView = findViewById(R.id.detail_name_text_view) | ||
detailPhoneNumTextView = findViewById(R.id.detail_phone_num_text_view) | ||
detailEmailTextView = findViewById(R.id.detail_email_text_view) | ||
detailBirthdayTextView = findViewById(R.id.detail_birthday_text_view) | ||
detailGenderTextView = findViewById(R.id.detail_gender_text_view) | ||
detailMemoTextView = findViewById(R.id.detail_memo_text_view) | ||
|
||
detailEmailLayout = findViewById(R.id.detail_email_layout) | ||
detailBirthdayLayout = findViewById(R.id.detail_birthday_layout) | ||
detailGenderLayout = findViewById(R.id.detail_gender_layout) | ||
detailMemoLayout = findViewById(R.id.detail_memo_layout) | ||
} |
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.
기본적으로 주석과 함수가 적절히 쪼개져 있는 것이 매우 보기 좋습니다.
다만 요러한 initViews 작업은 ViewBinding 을 사용한다면 코드 작성이 따로 필요 없이 간편하게 처리될 수 있을 것 같아요!
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를 사용하라고 하셔서 사용중인데 ViewBinding도 사용해보겠습니다!
val contact: Contact? = | ||
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.TIRAMISU) { | ||
intent.getParcelableExtra("contact", Contact::class.java) | ||
} else { | ||
intent.getParcelableExtra("contact") | ||
} |
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.
nullable, 버전 처리 매우 좋습니다!
lateinit var detailNameTextView: TextView | ||
lateinit var detailPhoneNumTextView: TextView | ||
lateinit var detailEmailTextView: TextView | ||
lateinit var detailBirthdayTextView: TextView | ||
lateinit var detailGenderTextView: TextView | ||
lateinit var detailMemoTextView: TextView | ||
lateinit var detailEmailLayout: ConstraintLayout | ||
lateinit var detailBirthdayLayout: ConstraintLayout | ||
lateinit var detailGenderLayout: ConstraintLayout | ||
lateinit var detailMemoLayout: ConstraintLayout |
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.
접근제한자를 습관적으로 선언하시는 것을 추천드립니다 :)
class ContactRecyclerViewAdapter( | ||
var contactList: ArrayList<Contact>, | ||
var inflater: LayoutInflater, | ||
val context: Context, |
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.
activity 를 변수로 넘기는 것은 금기시해야할 것 중 하나입니다. inflater 또한 파라미터로 받을 필요는 없을 것 같구요.
아이템을 클릭했을 때 이동해야한다면, 아이템이 클릭되었음을 알려주는 listener 를 정의하고 그것을 파라미터로 받는 것이 좋습니다.
interface OnContactClickListener {
fun onContactClicked(position: Int)
}
var inflater: LayoutInflater, | ||
val context: Context, | ||
) : RecyclerView.Adapter<ContactRecyclerViewAdapter.ViewHolder>() { | ||
inner class ViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) { |
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.
inner class 일 필요가 있을까요?
val lastNameTextView: TextView | ||
val nameTextView: TextView |
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.
외부에서 접근가능하도록 하는 것보다는 내부에서 set text 를 해줄 수 있는 메소드를 생성하심을 추천드립니다.
@Override | ||
override fun onSaveInstanceState(outState: Bundle) { | ||
super.onSaveInstanceState(outState) | ||
outState.putParcelableArrayList("contact_list", ArrayList(contactList)) |
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.
이러한 key 들은 const 로 따로 관리되는 편이 좋습니다.
해당 RegisterActivity 의 companion object 에 정의되어도 괜찮을 것 같구요.
이러한 형태로 수동으로 넣게된다면, 휴먼에러 덕분에 오류가날 가능성이 높습니다.
class RegisterActivity: AppCompatActivity() {
...
companion object {
const val KEY_CONTACT_LIST = "contact_list"
}
}
if 문에서 isVisible을 사용하는 방법으로 리팩토링 Co-authored-by: Victhor <[email protected]>
- Gender enum class 사용 - 가독성 및 입력 폼 로직 개선 - 접근 제한자 사용 - onBackPressedCallback 사용 - 리사이클러뷰 구조 개선 - key 관리 객체 추가
1단계 커밋 모음 링크
코드 작성하면서 어려웠던 점
생일 입력 폼을 구현할 때 캘린더에서 날짜를 선택해 등록하는 것을 처음 해봐서 좀 어려웠던 것 같습니다.
Intent로 data class 객체를 전달하는 방법을 잘 몰라서 구글링하면서 구현하였습니다.
코딩을 시작하기 전에 디자인, 기능 요구 사항들을 완벽히 이해하고 시작해야 코드를 다시 짜는 일이 줄어들 것 같습니다.
중점적으로 리뷰해주셨으면 하는 부분
커밋 단위가 적절한지 궁금합니다.
data class를 Intent로 전달하는 과정이 효율적으로 잘 구현이 됐는지 봐주시면 좋을 것 같습니다.
함수들이 적절하게 분리되어 사용되고 있는지 봐주시면 좋을 것 같습니다.
결과 화면