Search code examples
iosswiftuitableviewuikitgrand-central-dispatch

UITableViewCell image from url is wrong at first load


Situation

There's something wrong with setting url image to UITableViewCell. I created CancelableImageView which is custom UIImageView to cancel image downloading and setting task when new image is set. By using this method, "Wrong image" issue seems solved but this still happened to UITableView

After UITableView is loaded, everything looks great but after first cell finish loading image and change image, fourth cell's image also changed to first cell's image. And fourth cell's image changed to correct image after scrolling.

Code

This is my image loading extension for UIImage (Contains Image Caching)

/// Singleton for Image Caching
class ImageCacheManager {
    
    /// Storage to save cached UIImage
    private let cache = Cache<String, UIImage>()
    
    /// singleton instance
    static let shared = ImageCacheManager()
    
    /**
    Get image from cache data for url String
    - Parameter key:  String url of UIImage
    - Returns: Retrun cached image for url. Retrun nil when cached image is not exist
    */
    func loadCachedData(for key: String) -> UIImage? {
        let itemURL = NSString(string: key)
        return cache.value(forKey: key)
    }
    
    /**
    Save UIImage to cache data
     - Parameters:
        - image: UIImage to save in cache data
        - key:  String url of UIImage
     */
    func saveCacheData(of image: UIImage, for key: String) {
        let itemURL = NSString(string: key)
        cache.insertValue(image, forKey: key)
    }
}

extension UIImageView {

    /**
     Set image to UIImageView with Cached Image data or data from URL
     - Parameters:
        - urlString: String url of image
        - forceOption: Skip getting image from cache data and force to get image from url when true. default false
     */
    func loadImage(_ urlString: String?, forceOption: Bool = false) -> UIImage? {
        guard let imagePath = urlString else { return nil }
        
        // Cached Image is available
        if let cachedImage = ImageCacheManager.shared.loadCachedData(for: imagePath), forceOption == false {
            return cachedImage
            
            // No Image Cached
        } else {
            guard let imageURL = URL(string: imagePath) else { return nil }
            guard let imageData = try? Data(contentsOf: imageURL) else { return nil }
            guard let newImage = UIImage(data: imageData) else { return nil }
            
            ImageCacheManager.shared.saveCacheData(of: newImage, for: imagePath)
            return newImage
        }
    }
    
    func setImage(_ urlString: String?, forceOption: Bool = false) {
        DispatchQueue.global().async {
            guard let image = self.loadImage(urlString, forceOption: forceOption) else { return }
            
            DispatchQueue.main.async {
                self.image = image
            }
        }
    }
}

This is Custom UIImage that cancel image loading task when new image is set:

/// UIImageView with async image loading functions
class CancelableImageView: UIImageView {
    
    /// Cocurrent image loading work item
    private var imageLoadingWorkItem: DispatchWorkItem?
    
    override init(frame: CGRect) {
        super.init(frame: frame)
    }
    
    required init?(coder aDecoder: NSCoder) {
        super.init(coder: aDecoder)
    }
    
    init() {
        super.init(frame: CGRect.zero)
    }
    
}

// MARK: Concurrent image loading method
extension CancelableImageView {
    
    /**
     Cancel current image loading task and Set image to UIImageView with Cached Image data or data from URL
     - Parameters:
        - urlString: String url of image
        - forceOption: Skip getting image from cache data and force to get image from url when true. default false
     */
    func setNewImage(_ urlString: String?, forceOption: Bool = false) {
        
        self.cancelLoadingImage()
        self.imageLoadingWorkItem = DispatchWorkItem { super.setImage(urlString, forceOption: forceOption) }
        self.imageLoadingWorkItem?.perform()
    }
    
    /// Cancel current image loading work
    func cancelLoadingImage() {
        DispatchQueue.global().async {
            self.imageLoadingWorkItem?.cancel()
        }
    }
}

This is my UITableViewCell for that view:

class ChartTableViewCell: UITableViewCell {

    lazy var posterImageView = CancelableImageView().then {
        $0.contentMode = .scaleAspectFit
        $0.image = UIImage(named: "img_placeholder")
    }
    
    ... 

    override func prepareForReuse() {
        setPosterPlaceholderImage()
        super.prepareForReuse()
    }
}

extension ChartTableViewCell {
    
    /// Set poster imageView placeholder Image
    private func setPosterPlaceholderImage() {
        self.posterImageView.image = UIImage(named: "img_placeholder")
    }
    
    // MARK: Set Data
    func setData(rank: Int, movie: MovieFront) {
        
        rankLabel.text = "\(rank+1)"
        titleLabel.text = movie.title
        genreLabel.text = movie.genre
        releaseDateLabel.text = movie.releaseDate
        starRating.rating = movie.ratingScore/2
        ratingCountLabel.text = "(\(movie.ratingCount))"
        if let imagePath = movie.posterPath {
            posterImageView.setNewImage(APIService.configureUrlString(imagePath: imagePath))
        }
    }
}

This is UITableViewDataSource (cellForRowAt):

    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        
        guard let cell = tableView.dequeueReusableCell(withIdentifier: identifiers.chart_table_cell) as? ChartTableViewCell else {
            return UITableViewCell()
        }
        
        cell.setData(rank: indexPath.row, movie: viewModel.movieListData.value[indexPath.row])
        return cell
    }

Solution

  • A couple of observations:

    1. In setImage, the asynchronous self.image = image is problematic because you don’t know if the cell (and therefore the image view) has been reused for another row/item in your table/collection view. I know you think you have canceled it, but you haven’t. See point 2, below.

      The general solution (in a UIImageView extension is to have an “associated value” (e.g., objc_setAssociatedObject(_:_:_:_:), etc.) for the URL and make sure that the URL for this particular image view is still the same URL that you just fetched asynchronously before updating the image property. Of if you really want to use the subclass approach, just store this in some property of you subclass (and lose the UIImageView extension entirely). The elegant solution is to not have any image view subclass, but put it a UIImageView extension, and use “associated values” for any additional values you need to keep track of for a particular image view instance.

    2. You are canceling the dispatch work item, but are not actually ever canceling the request. (Cancelation is “cooperative”, not “preemptive”.) Worse, you aren’t even using a cancelable network request.

      I would suggest that rather than a dispatch work item and Data(contentsOf:), use URLSession and save the URLSessionTask. That is cancelable. And, again, we would use as “associated value” to save the old URLSessionTask in case you need to cancel it. See https://stackoverflow.com/a/45183939/1271826.

    3. When you initiate an asynchronous image retrieval, make sure to reset the image back to the placeholder, in case the cell in question has been reused. I see you setting the placeholder when the image view is first created, but not when fetching a new image for a reused cell.

    In full disclosure, the above are general observations about fixing problems in your asynchronous image retrieval/cacheing capabilities for an image view, but you might have yet another problem here, too. Specifically, the above is fixes problems arising from scrolling cells in and out of view, but your animated gif is showing yet another problem, where updating the first cell’s image appears to be updating the fourth cell’s image. This could be a manifestation of the third bullet point above, or it could be something else. There is not enough here to diagnose it. I’d suggest fixing the above issues, and if you are still having problems, then post a new question with a MRE to help diagnose this further.

    But, frankly, I might encourage you to consider establish asynchronous image retrieval and caching libraries, such as KingFisher, SDWebImage, AlamofireImage, etc. All of this is sufficiently complicated that it might be prudent to not “reinvent the wheel”.