getditto/demoapp-chat

💉Move Ditto to DittoChatApp Hilt AndroidApp class

okdistribute opened this issue · 4 comments

💉Move Ditto to DittoChatApp Hilt AndroidApp class
abunur commented

@okdistribute can you remind me what this ticket was about, if you can recall? thanks

Thanks, we want to be able to show customers the best practice with using Hilt with ditto, so that even a sales engineer with little experience with Android specifically could link to your implementation as an example.

In our call on 3/13 where we did a code audit of the existing android chat app, you mentioned that this was still not implemented with best practice and you wanted to move the ditto code into the hilt class. Is this still the case?

abunur commented

Thanks, we want to be able to show customers the best practice with using Hilt with ditto, so that even a sales engineer with little experience could link to your implementation as an example.

In our call on 3/13 where we did a code audit of the existing android chat app, you mentioned that this was still not implemented with best practice and you wanted to move the ditto code into the hilt class. Is this still the case?

ok thanks for clarifying. The Chat tutorial is using Hilt for DI (e.g. the ViewModel is a Hilt ViewModel and provided via DI, and Data Store, the new way of doing shared prefs, is also provided via DI). However, I would like to provide the Ditto instance through injection rather than the way we currently provide it. The actual setup for Ditto is done in an activity - it's not even done at the Application class level.

@phatblat do you have any thoughts about this? I think you know a lot more about how the back end of the Android Ditto code is implemented. Is there a clean way to produce the Ditto instance, e.g. via a Factory, and then provide it via injection?

On second thought maybe this can be solved within a Hilt Module. I'll take a run at it and update this ticket.

thanks

abunur commented

@okdistribute this is in backlog, has not been addressed yet. Not needed for OSS, either, maybe we can move to different Epic if it's under that one?