dio icon indicating copy to clipboard operation
dio copied to clipboard

Published 20 hours ago • verified publisher icon cfug

Remove DioMixin

Open kuhnroyal opened this issue 1 year ago • 4 comments

Just a quick hack to figure out if we can get rid of DioMixin and if that would actually make sense. The DioMixin is currently the main class. The DioForNative and DioForBrowser classes contain minimal logic for download handling. We can extract this logic into some new platform specific DownloadAdapter in the same way the HttpClientAdapter works. At the same time we can combine the DioMixin and Dio classes. This should overall reduce the complexity of having 4 different Dio* classes

I think this would be a good change, not totally happy with passing 2 more parameters to the download handler yet. What do you think @cfug/devs?

New Pull Request Checklist

  • [ ] I have read the Documentation
  • [ ] I have searched for a similar pull request in the project and found none
  • [ ] I have updated this branch with the latest main branch to avoid conflicts (via merge from master or rebase)
  • [ ] I have added the required tests to prove the fix/feature I'm adding
  • [ ] I have updated the documentation (if necessary)
  • [ ] I have run the tests without failures
  • [ ] I have updated the CHANGELOG.md in the corresponding package

Additional context and info (if any)

kuhnroyal avatar Dec 28 '23 15:12 kuhnroyal

I actually like this

ueman avatar Dec 28 '23 19:12 ueman

@AlexV525 Would also love your opinion on this idea.

kuhnroyal avatar Jan 10 '24 21:01 kuhnroyal

@kuhnroyal Sorry for the late review. I've looked at why DioMixin was introduced and it seems to be the idea to implement platform-specific requests, starting from here: https://github.com/cfug/dio/commit/632931586b5153fb03f58a75378e59b564276b82#diff-481f5993a7795ed1ee31f808d11a579a48fd60d698e8ca13f85dcb3acff5d421

So basically my two cents:

  1. Removing DioMixin looks great at this point, we have already done this (platform-specific implementation) in many ways.
  2. mixin is still useful. It would be great if the DownloadAdapter could somehow convert to mixin class DioDownloadMixin on Dio, and maybe to avoid duplicate parameters.

AlexV525 avatar Jan 11 '24 01:01 AlexV525

2. mixin is still useful. It would be great if the DownloadAdapter could somehow convert to mixin class DioDownloadMixin on Dio, and maybe to avoid duplicate parameters.

Not sure we can mixin a platform dependent mixin class. Also the adapter pattern allows for custom implementations.

kuhnroyal avatar Jan 11 '24 14:01 kuhnroyal