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

Handle network response in datamanager #11

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ISatya
Copy link
Contributor

@ISatya ISatya commented Feb 22, 2017

DataManager does not need to implement ApiHelper. This removes unnecessary implementation of getApiHeader.
Helper methods to perform login can be extracted to a separate interface(LoginHelper) and implemented by datamanager. Datamanager will delegate the login tasks to Apihelper.

Presenter only needs to know if login was successful or some error occurred. So I've just returned an emtpy observable in case of successful response.

@janishar
Copy link
Owner

@ISatya
Thank you. We highly appreciate your insights.

I have few concern in this approach.

  1. The DataManager code will become huge and unmanageable when the entire application start to take shape.
  2. If the presenter have to delegate the data from network to the view then there will be unnecessary interception by the DataManager. DataManager will do nothing with that data but to extract and then forward it to the presenter.
  3. In the above implementation the DataManager is handling the business logic of whether to logout user or log in. The business logic should be only handled by the presenter. DataManager should only be responsible for handling the data as told by the presenter.

Thanks

@ISatya
Copy link
Contributor Author

ISatya commented Feb 22, 2017

  1. If the entire application data is handled by a single DataManager then it will get bloated. DataManager should be split into multiple managers.
    In current implementation, all tasks are implemented in a Single DataManager.
    If we extract login related tasks to a separate manager - LoginDataManager and do the same with any new modules that are added, individual data managers will be small and manageable.

  2. Agreed. But in this case, presenter is responsible for handling network response. This adds additional responsibility of forwarding response to DataManager.
    If my understanding is correct, DataManager should be responsible of managing data. It should decide what to do with the response, whether to persist it or just pass it to the presenter or to convert it to a form required by the presenter. Presenter should not be forwarding network response. It should receive data from DataManager and use it to decide
    what the view should show.

  3. Presenter is the one initiating logout or login. Presenter tells DataManager to perform logout call and DataManager does the required network calls, updating user session locally and then inform presenter whether logout was successful or not.

Thanks

@amitshekhariitbhu
Copy link
Contributor

@ISatya Agreed with most of your points. We will think on this and discuss.

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.

3 participants