Skip to content

Mani: QuotifyApp - Remote call #38

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

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

itmani1991
Copy link
Contributor

@itmani1991 itmani1991 commented Oct 7, 2024

Pull Request

Description

Lambda func like CopyText & ShareAction events moved into MainEvents(Prev MR comment addressed)
State passed into screens instead data(Prev MR comment addressed)
Toast handled with extension func and event
Remote call implementation with retrofit
(Updated)
Removed GSON converter
Kotlin Serialization technique implemented
Manual dependency items simplified
Removed static data list
Handled the repetative livedata approach into sharedflow with Event.
Added Okhttp loggin interceptor for logs of HTTP requests and responses

Checklist

  • My project includes a README.md file with a brief overview of the project, how to set it up, and any relevant information.
  • I have added a screenshot or screen recording showing the functionality of the app.
  • I have tested the project and confirmed it works as expected.

Screenshots

Project README

Please ensure that your project's README.md is detailed and includes:

  • A brief introduction to the project.
  • Installation steps.
  • Key features.
  • Any additional instructions or information.

Additional Notes


Thank you for your contribution!

- lambda func like CopyText & ShareAction events moved into MainEvents
- state passed into screens instead data
- Toast handled with extension func and event
- Remote call implementation
@@ -63,6 +63,8 @@ dependencies {
implementation(libs.androidx.ui.navigation)
implementation(libs.androidx.room.runtime)
implementation(libs.androidx.room.ktx)
implementation(libs.retrofit)
implementation(libs.converter.gson)
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use kotlin serialization?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved


import com.mani.quotify007.domain.model.Quote

val quotesDataList = mutableListOf(
Copy link
Contributor

Choose a reason for hiding this comment

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

do we still need these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

Comment on lines 31 to 39
viewModel.copyTextEvent.observe(this) { quote ->
onCopyText(this, quote)
}
viewModel.shareClickEvent.observe(this) { quote ->
shareQuote(this, quote)
}
viewModel.showToast.observe(this) { message ->
message?.let { showToast(it) }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

more better approach? looks repetative

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

Comment on lines 23 to 31
private val _copyTextEvent = MutableLiveData<Quote>()
val copyTextEvent: LiveData<Quote> = _copyTextEvent

private val _shareClickEvent = MutableLiveData<Quote>()
val shareClickEvent: LiveData<Quote> = _shareClickEvent

private val _showToast = MutableLiveData<String>()
val showToast: LiveData<String> = _showToast

Copy link
Contributor

Choose a reason for hiding this comment

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

can't we use just MainEvent

Copy link
Contributor

Choose a reason for hiding this comment

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

Alos, why livedata, why not stateflow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

-Removed GSON converter
-Used Kotlin Serialization
-Manual dependency items simplified
@itmani1991
Copy link
Contributor Author

@anandwana001 Addressed comments, please review.

Comment on lines 34 to 37
_state.value =
_state.value.copy(favoriteQuotes = quotes)
_state.value = _state.value.copy(
quotes = useCase.result().results.map { quote ->
Copy link
Contributor

Choose a reason for hiding this comment

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

why multiple, why not one statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved

@itmani1991
Copy link
Contributor Author

@anandwana001 Comment addressed. Please review.

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