Completion Block should contain the url for downloadWithURL

[ilushka85]

The current completion block looks like this:

completed:^(UIImage *image, NSError *error, SDImageCacheType cacheType, BOOL finished)

It really should pass back the original URL. This is neccessary when using the sdwebimagedownloader in a reusable cell. Basically since a cell is reused and completion of image download is async the download of a different image finishes when we are already loading a next one or loaded next image from cache there by replacing the image we should display. If the url were passed here we could check against the original url to see if we should use(display) this image that finished.

[bpoplauschi]

There is a pull request with this #759. But even without that, you can retain the url in a UITableViewCell property and compare it with the one you used for the request.

[ilushka85]

Yes this is my push request to fix this issue. You cannot compare it to the url even if its retained in the cell since the completion happens asynchronously ... You need the url in the completion block to compare against the retained url in the uitableviewcell to see if the image should be set or not.

[bpoplauschi]

I think you can check it, since the issue is the cell that started the request might get reused before the response arrives. I would do something like

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {
   ...
   cell.imageUrl = url;
   __weak typeof(cell)weakCell = cell;
   ...
   [cell.customImageView setImageWithURL:url completed:^(UIImage *image, NSError *error, SDImageCacheType cacheType) {
      __strong __typeof(weakCell)strongCell = weakCell;
      if ([strongCell.imageUrl isEqual:url]) {
         strongCell.customImageView.image = image;
      }
   }
}

[ilushka85]

Without testing this it may work.... but its a total hack / workaround for something that truly makes sense to be in the completion block.

[bpoplauschi]

I was giving you an option until that gets merged in.

[ilushka85]

@bpoplauschi Thank you but since i created the pull request its merged on our end so we are ok for now. I just wanted to ensure this gets included some time in the future.

[bpoplauschi]

👍

[ilushka85]

@bpoplauschi any idea on timeframe on the merge? I know its a breaking change so just curious.

[bpoplauschi]

It will probably get included in the next version. If we can't find a backwards compatible way, we might need to bump to version 4.x

[ilushka85]

What about if i sent a push update that maintains backwards compatibility by creating some stub methods to reference the old completion blocks? That way you can call with new one or old one.

[bpoplauschi]

Go ahead and try it. We will need a way to know which completion blocks to call and with how many parameters.

[bpoplauschi]

Fixed by #770