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

Inject view model on activity #23

Merged
merged 15 commits into from
Jun 2, 2023

Conversation

HectorNarvaez
Copy link
Contributor

@HectorNarvaez HectorNarvaez commented May 31, 2023

Added code for perform injection of the view model into the FileViewerActivity

In this PR you will find:

  • Added on sample app implementation of activity ktx
  • Implementation for provide coroutines scopes
  • Implementation of base activity
  • Implementation for base view model
  • Added abstractions for view event and view state
  • Small fix into GoogleStorageApi-
  • UI improvements into file viewer activity
  • Implement extension function for launch coroutines in a safer way

Evidence:

Screenshot_20230531_163457

Screenshot_20230531_162708

tvSortByName.setOnClickListener {
// viewModel.sortByName()
private fun initializeAdapter() {
if (filesAdapter == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be an early return. That would help reduce the indentation levels

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think in this particular case, an early return will not reduce indentation levels. It would be something like:

    if(filesAdapter != null) {
        return
    } else {
        filesAdapter = FileGridAdapter(this)
        with(binding.filesRecyclerView) {
            layoutManager = GridLayoutManager(this@FileViewerActivity, 2)
            adapter = filesAdapter
        }
    }

which basically is the same

Copy link
Contributor

@Anwera64 Anwera64 Jun 2, 2023

Choose a reason for hiding this comment

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

Not really, that's not an early return. It would look something like this:

if (filesAdapter != null) {
    return
}
filesAdapter = FileGridAdapter(this)
with(binding.filesRecyclerView) {
    layoutManager = GridLayoutManager(this@FileViewerActivity, 2)
    adapter = filesAdapter
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i think in general use inverse logic in a conditional makes code less readable than just do a straight comparison.
Anyway, i applied the suggested change 😃

done ✅

Copy link
Contributor

Choose a reason for hiding this comment

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

I disagree. Early returns serve as precondition checks that show you what would stope the code from running in the first place. If you would have multiple validations, they would all go first and show you all the failure cases for the function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, i'm ok with early returns. I mean about use inverse logic. I think is more readable use if(boolean) instead if(!boolean)

Copy link
Contributor

@Anwera64 Anwera64 left a comment

Choose a reason for hiding this comment

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

LGTM

@HectorNarvaez HectorNarvaez merged commit c7ee7f4 into develop Jun 2, 2023
@HectorNarvaez HectorNarvaez deleted the state/injectViewModelOnActivity branch June 2, 2023 19:35
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