search

pinterest / PINRemoteImage

A thread safe, performant, feature rich image fetcher
Apache License 2.0
4.01k stars 511 forks source link

Add cancel method for PINRemoteImageManager #509

Closed zhongwuzw closed 5 years ago

zhongwuzw commented 5 years ago

Add cancel all tasks for PINRemoteImageManager, it's the convenience method for user to call all tasks one shot, don't need to maintain uuids by themself.

ghost commented 5 years ago

🚫 CI failed with log

bolsinga commented 5 years ago

Please add tests.

garrettmoon commented 5 years ago

Hmm, I'm not sure this will work. If the UUIDs are stored in a weak hash table, PINRemoteImageManager will 'lose' references to requests that the caller doesn't strongly retain the UUID to…

zhongwuzw commented 5 years ago

@garrettmoon 🤔 Emm, PINRemoteImageTask would retain UUID by _callbackBlocks.

zhongwuzw commented 5 years ago

@garrettmoon And I may have a thought to optimize _locked_taskForUUID:key: of PINRemoteManager by add a weak to weak NSMapTable(uuid to task) and exposure key of PINRemoteImageTask, what do you think? 🤔

garrettmoon commented 5 years ago

@garrettmoon 🤔 Emm, PINRemoteImageTask would retain UUID by _callbackBlocks.

You're right! Would you mind adding a comment to the code saying as much?

garrettmoon commented 5 years ago

@garrettmoon And I may have a thought to optimize _locked_taskForUUID:key: of PINRemoteManager by add a weak to weak NSMapTable(uuid to task) and exposure key of PINRemoteImageTask, what do you think? 🤔

Seems like a good optimization because it will also make the code clearer I think!

zhongwuzw commented 5 years ago

@garrettmoon And I may have a thought to optimize _locked_taskForUUID:key: of PINRemoteManager by add a weak to weak NSMapTable(uuid to task) and exposure key of PINRemoteImageTask, what do you think? 🤔

Seems like a good optimization because it will also make the code clearer I think!

👌 I'll make a PR later~