Drutol / MALClient

Not so small client app for Myanimelist.net - Windows 10 UWP & Android
GNU General Public License v3.0
352 stars 32 forks source link

Minor Http Improvements #245

Closed projectgoav closed 4 years ago

projectgoav commented 5 years ago

Don't know if you accept external PRs or not so feel free to close off :)

There are quite a few places that use HttpClient without disposing of resources etc that I've not looked at yet. If you're happy with these changes I can submit updates or another PR for them.

Changes

Breakages

I don't believe this should break anything on any platform you support. The stuff I've added is all quite low level and exposes the same api to the upper layers.

Only thing I can think may effect things is the use of ConfigureAwait(false). I'm not entirely sure about async/await code as I find TaskContinuations easier to code and read.... I'm sure I've read that using ConfigureAwait(false) is considered best practice? But as this is done low level I'm wondering if this breaks any assumptions made in the upper layers about what thread/context async stuff is running on. Any thoughts?

Drutol commented 5 years ago

Uff man, I salute for wanting to play in this spaghetti landfill and sorry for this code ^^

Essentially it all should be rewritten but I don't find myself with enough time nor motivation given how MAL works.

projectgoav commented 5 years ago

I guess small improvements here and there will get there in the end?

There's a few other places I've fixed a couple of memory leaks and issues I was going to submit soon. If you'd rather re-write bits then I'm happy to help out with some of it rather than work on patching exisiting code?