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

Why is RestAdapter Singleton and not a dependency ? #1

Closed
pdegand opened this issue Apr 27, 2015 · 3 comments
Closed

Why is RestAdapter Singleton and not a dependency ? #1

pdegand opened this issue Apr 27, 2015 · 3 comments

Comments

@pdegand
Copy link

pdegand commented Apr 27, 2015

Hello,

Great article about MVP + Dagger 2 + Retrofit. Helped me a lot.

I just have a question about the RestAdapter. In the current version of the code, the RestAdapter is a self implemented Singleton (with some flaws in the impl tbh) and not a dependency provided by Dagger ?

Why did you choose to do like this and did not put this dependency in the InteractorModule ?
The RestAdapter could be injected in the constructor of the FindItemsInteractorImpl and you will have the behavious properly implemented with Dagger2.
Or maybe i'm missing a point of Dagger and/or Retrofit that prevents you to do such things.

Thank you

@fr4nk1
Copy link
Owner

fr4nk1 commented Apr 27, 2015

Hello Pierre,

I'm glad that my code helps you.

Indeed your aproach is correct. RestAdapter must be in the InteractorModule
because many interactors will use RestAdapter.

If I have a free moment this week, I will update the code.

Thanks for your time and to report me this.

Best regards.
-Fran.

2015-04-27 8:49 GMT+02:00 Pierre Degand notifications@github.com:

Hello,

Great article about MVP + Dagger 2 + Retrofit. Helped me a lot.

I just have a question about the RestAdapter. In the current version of
the code, the RestAdapter is a self implemented Singleton (with some flaws
in the impl tbh) and not a dependency provided by Dagger ?

Why did you choose to do like this and did not put this dependency in the
InteractorModule ?
The RestAdapter could be injected in the constructor of the
FindItemsInteractorImpl and you will have the behavious properly
implemented with Dagger2.
Or maybe i'm missing a point of Dagger and/or Retrofit that prevents you
to do such things.

Thank you


Reply to this email directly or view it on GitHub
#1.

@vwmattr
Copy link
Contributor

vwmattr commented May 1, 2015

I added PR 2 to inject the RestAdapter in the InteractorModule. Took it as an opportunity to try and test my understanding of what I learned from the blog post and this repo. Please let me know if that is what you had in mind @fr4nk1 or if there's something you'd change to improve that.

@fr4nk1
Copy link
Owner

fr4nk1 commented May 2, 2015

I like your approach @vwmattr . I am thinking in reuse the method in other interactors and it's good.
@pdegand I going to close this issue, this approach resolve the problem ;)

@fr4nk1 fr4nk1 closed this as completed May 2, 2015
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

No branches or pull requests

3 participants