-
Notifications
You must be signed in to change notification settings - Fork 3
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
정렬 기능 추가 #59
base: CJE
Are you sure you want to change the base?
정렬 기능 추가 #59
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.
코틀린 스타일 가이드
스타일 가이드 참고해서 괄호 관련 띄어쓰기를 신경써보시는 것도 추천드립니다 ㅎㅎ
@@ -12,7 +12,7 @@ import com.gdsc.todo.model.dao.ToDoDao | |||
// 반드시 RoomDatabase를 상속받은 추상클래스를 만들어야 한다. | |||
// 추상 클래스로 만든 인스턴스는 주로 싱글톤으로 만든다. | |||
// 같은 시간에 여러 개의 인스턴스에서 데이터베이스에 접근하는 것을 막기 위함이다. | |||
@Database(entities = [MyToDoList::class], version = 6) | |||
@Database(entities = [MyToDoList::class], version = 9) |
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.
앱을 재실행하지 못하고 재설치를 해야되는 에러가 생기지는 않았나요?
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.
룸 마이그레이션
SQL문을 거쳐서 마이그레이션하는 방식도 있습니다.
when(titleIsSorted || dateIsSorted){ | ||
true -> toDoAdapter = ToDoAdapter(viewModel.sortMyToDoSet, viewModel) | ||
false -> toDoAdapter = ToDoAdapter(viewModel.myToDoSet, viewModel) | ||
} |
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.
여기서 true로 가는 경우가 있나요?
// toolbar에 추가된 항목 클릭 시 이벤트 처리 | ||
override fun onOptionsItemSelected(item: MenuItem): Boolean { | ||
when(item.itemId){ | ||
R.id.sort_title -> { | ||
when(titleIsSorted) { | ||
true -> { | ||
Log.d(ISSORTED, titleIsSorted.toString()) | ||
titleIsSorted = false | ||
setRecyclerView() | ||
} | ||
false -> { | ||
Log.d(ISSORTED, titleIsSorted.toString()) | ||
viewModel.sortTitle() | ||
titleIsSorted = true | ||
dateIsSorted = false | ||
setRecyclerView() | ||
} | ||
} | ||
return true | ||
} | ||
R.id.sort_date -> { | ||
when(dateIsSorted){ | ||
true -> { | ||
dateIsSorted = false | ||
setRecyclerView() | ||
} | ||
false -> { | ||
viewModel.sortDate() | ||
dateIsSorted = true | ||
titleIsSorted = false | ||
setRecyclerView() | ||
} | ||
} | ||
return true | ||
} | ||
} | ||
return super.onOptionsItemSelected(item) | ||
} |
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.
여기도 세분화하면 좋을 것 같아요.
when(item.itemId) {
R.id.sort_title -> abcd(titleIsSorted)
R.id.sort_date -> qwer(dateIsSorted)
}
처럼 표현하면 가독성이 좋아지지 않을까요?
// _sortMyToDoSet = _myToDoSet 얕은 복사(기존 객체에 영향 O) | ||
_sortMyToDoSet.addAll(_myToDoSet) // 깊은 복사(기존 객체에 영향 X) |
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.
삽질하기 좋은 개념인데 잘 이해가 되셨길ㅎㅎ
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.
Cloneable
android:layout_height="0dp" | ||
android:layout_weight="1" | ||
android:text="@{item.date}" | ||
android:textColor="#C67C5252" |
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.
프로젝트 규모가 커진다면 color도 extract해야 수정할 때 번거롭지 않습니다. 또한 색상을 잘못고르는 실수가 없죠
하지만 extract하는 과정 자체가 번거롭기 때문에 조삼모사 같다는 생각도 해요.
리소스 파일에 대한 관리도 가볍게 고려해보셔도 좋을 것 같습니다.
저는 아래 링크 참고해서 프로젝트 진행했었어요~
헤이딜러 컨벤션
|
||
class ToDoActivity : AppCompatActivity() { | ||
private lateinit var recyclerView: RecyclerView | ||
private lateinit var toDoAdapter: ToDoAdapter | ||
private lateinit var viewModel: ToDoViewModel | ||
private lateinit var toolbar: androidx.appcompat.widget.Toolbar |
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.
이건 취향 차이일 수 있겠는데, toolbar를 굳이 변수로 둬야하나 싶네요.
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.
LGTM
What is this pr
Changes
Screenshot