Skip to content

Milestone5#3

Open
Yuki-YuXin wants to merge 3 commits into
mainfrom
milestone5
Open

Milestone5#3
Yuki-YuXin wants to merge 3 commits into
mainfrom
milestone5

Conversation

@Yuki-YuXin
Copy link
Copy Markdown
Owner

remove databinding

@Yuki-YuXin Yuki-YuXin requested a review from SammyO December 8, 2022 06:20
.setTitle("Choose the filtering options:")
.setView(com.example.assignment.R.layout.view_sort_dialog)
.setPositiveButton(
"Apply"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't use hardcoded strings, use string resources instead.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This is not addressed yet.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

In commit resolve milestone5 comments
It has been changed to R.string.main_dialogue_title but back to "Choose the filtering options:" in milestone 6 in the latest commit "repository with internal storage for data persistence". Will check all comments in miletone1-milestone5 in milestone6's code.


if (radioAscendingButton.isChecked) {
when (compareText) {
"Mass" -> sortList = viewModel.meteors.value?.sortedBy { it.mass }?.toList() ?: emptyList()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Part of his logic belongs in the ViewModel. This function should be responsible for handling the click event (by looking at the ID of the selected item), and then calling the ViewModel to inform it that a sorting has happened. You can consider introducing a sort type enum: this function would call the ViewModel with something like sortingSelected(SortType.MASS), and the sortingSelected()function in the ViewModel then updates the list throughsortedBy`, and pass the new list back to the Activity (through LiveData).

val meteors: LiveData<List<MeteorData>> = _meteors

init {
getMeteorsInfo()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We discussed this before - this call shouldn't happen on ViewModel class initialisation, but on some kind of Activity life cycle event (e.g. onResume).

try {
_meteors.value = MeteorsApi.retrofitService.getMeteorsInfo()
} catch (e: Exception) {
_meteors.value = listOf()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Consider adding an error state so that the user knows what's going on.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Did that in milestone 6


override fun onResume() {
super.onResume()
viewModel.refresh()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Don't do this, but do https://betterprogramming.pub/empowered-lifecycle-aware-viewmodel-for-android-f495de9a8170 instead (more automatic, and less manual work :))

meteorDataList.removeAt(pos)
notifyItemRemoved(pos)
}
fun update(data: List<MeteorData>) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where does this get called?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Where does this get called?

Milestone1 use MutableList without databinding. Milestone5 uses List so they never get called and removed here.

@Yuki-YuXin Yuki-YuXin force-pushed the milestone5 branch 2 times, most recently from e5285d3 to c3b534a Compare December 14, 2022 00:39
val radioAscendingButton = dialog.findViewById<View>(com.example.assignment.R.id.radioAscending) as RadioButton
val compareText = fieldSpinner.getSelectedItem().toString()

Hawk.init(this.context).build()
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What is Hawk?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

A simple key-value storage for android https://github.com/orhanobut/hawk

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