2

So guys, well I used to have the Massive View Controller problem, specially when using UITableView, now im changing an old app to be more modal. Before, I used to set the whole cell at cellForRowAt indexPath. set labels, download image etc.

Now I'm using the correct approach to set at the UITableViewCell subclass. but I'm having the problem of either, reuse cells where leaving the old pic while updating/downloading the new one, which I think I fixed, but I also noted it happens to be kinda putting the wrong image in the wrong cell and then updating to the correct one when doing a fast scroll or so...

Before I could do something like this to make sure the image is updated in the correct cell. that's when setting the whole cell from the view controller like this...

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {

    let cell = tableView.dequeueReusableCell(withIdentifier: "Identifier")

    cell.title = "Cool title"

    imageLoader.obtainImageWithPath(imagePath: viewModel.image) { (image) in
        // Before assigning the image, check whether the current cell is visible for ensuring that it's right cell
        if let updateCell = tableView.cellForRow(at: indexPath) {
            updateCell.imageView.image = image
        }
    }    
    return cell
}

Now I pretty much only have this at the viewcontroller...

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {

    let cell : RestCell = self.tableView.dequeueReusableCell(withIdentifier: "Cell", for: indexPath) as! RestCell

    // Pass data to Cell :) clean the mess at the View Controller ;)
    cell.restData = items[indexPath.row]

    return cell
}

And the UITableViewCell subclass I have this.

class RestCell: UITableViewCell {

    @IBOutlet var img : UIImageView!
    @IBOutlet var lbl : UILabel!
    @IBOutlet var lbl_tipocomida: UILabel!
    @IBOutlet var lbl_provincia: UILabel!
    @IBOutlet var img_orderonline: UIImageView!
    @IBOutlet var img_open: UIImageView!
    @IBOutlet var imgalldelivery: UIImageView!
    @IBOutlet var img_reservasonline: UIImageView!

    // get data and update.
    var restData: RestsModel!  {
        didSet {
            // default img early placeholder set. Eliminates showing old pic in cell reuse
            self.img.image = UIImage(named: "60x60placeholder.png")
            // Update all values func
            updateUI()
        }
    }

    override func awakeFromNib() {
        super.awakeFromNib()
        // Default image placeholder
       //self.img.image = UIImage(named: "60x60placeholder.png")

    }
    // required init.
    required init(coder aDecoder: NSCoder) {
        super.init(coder: aDecoder)!

        // Color of selected.
        let myCustomSelectionColorView = UIView()
        myCustomSelectionColorView.backgroundColor = UIColor(red: 234.0/255.0, green: 46.0/255.0, blue: 73.0/255.0, alpha: 0.1) //UIColor(netHex:0xFFFF434)
        selectedBackgroundView = myCustomSelectionColorView   
    }   

    fileprivate func updateUI() {

        if (self.restData.image != nil  &&  self.restData.image != "") {
            ImageLoader.sharedLoader.imageForUrl(urlString:self.restData.image, completionHandler:{(image: UIImage?, url: String) in
                self.img.contentMode = UIViewContentMode.scaleAspectFit
                self.img.image = image
            })
        } else {
            self.img.image = UIImage(named: "60x60placeholder.png")
        }

        self.lbl.text = restData.name as String
        self.lbl_provincia.text = restData.provincia as String
        self.lbl_tipocomida.text = restData.tipocomida as String

        // check if has alldelivery enabled, show Alldelivery logo
        if( self.restData.reservaonline == "1") {
            self.img_reservasonline.isHidden = false
            self.img_reservasonline.image = UIImage(named: "icon_reserva_on")

        } else {
            self.img_reservasonline.isHidden = true
            self.img_reservasonline.image = UIImage(named: "icon_reserva_off")
        }

        // check if has orderonline, show ordeonline text
        if ( self.restData.orderonline == "1") {
            self.img_orderonline.isHidden = false
        } else {

            self.img_orderonline.isHidden = true
        }

        // check if has schedule, display if open or close

        if (self.restData.hasSchedule == true) {

            if ( self.restData.openNow == true) {
                self.img_open.image = UIImage(named:"icon_open")
            }  else {
                self.img_open.image = UIImage(named:"icon_close")
            }

            self.img_open.isHidden = false
        } else {

            self.img_open.isHidden = true
        }

    }

}

I'm calling the download on the image from the UITableViewVCell subclass and setting it there. How can I make sure the image is being set to the correct Cell only???

Haroldo Gondim
  • 7,725
  • 9
  • 43
  • 62
lorenzo gonzalez
  • 1,894
  • 4
  • 14
  • 18
  • 1
    Compare with this question: https://stackoverflow.com/questions/45316099/nscache-doesnt-work-with-all-images-when-loading-for-the-first-time – 3stud1ant3 Oct 24 '17 at 02:22

2 Answers2

2

This is happening due to a race condition (this is very common)

Based on your current solution this is what I would do:

  • Remove the previous image when you reload the cell's data. You can just do something like self.myUIImage.image = nil after loading the new one.
  • Check if the cell is still visible after the image is downloaded:
    • if it is loaded, then adds the image.
    • if not then ignore it. It will be cached the next time you want to use it if you are caching it (this would be another topic).

You may want to check some similar questions like this one:

Images in UITableViewCells are loading wrong

Gabriel Goncalves
  • 5,132
  • 3
  • 25
  • 34
  • Hey gabox, like showed in my question i know how to check if the cell is the correct *visible* one when using the *set all in the view controller approach, but how can i do that from the uitableviewcell subclass ?? – lorenzo gonzalez Oct 24 '17 at 12:05
  • You need to have a delegate (remember it should be weak) to your table view – Gabriel Goncalves Oct 24 '17 at 13:33
  • you are going to kill me for this.. can you elaborate a full example ?. im finding mostly the other way around, a delegation so the cell actionates something in the view controller... – lorenzo gonzalez Oct 24 '17 at 14:27
  • can you give any example for this point? Check if the cell is still visible after the image is downloaded: if it is loaded, then adds the image. if not then ignore it. It will be cached the next time you want to use it if you are caching it (this would be another topic). – Avendi Sianipar Dec 24 '19 at 10:41
2

I recommend you to use 3rd party library to load image with url such as Kingfisher. It will handle caching and cancel loading for you.

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    let data = dataSource[indexPath.row]
    let cell = tableView.dequeueReusableCell(withIdentifier: "Identifier") as! ACell
    cell.update(data)
    return cell
}

And the cell will look like this.

import Kingfisher
class ACell: UITableViewCell {
    @IBOutlet fileprivate weak var backgroundImageView: UIImageView!

    fileprivate func reset() {
        backgroundImageView.image = nil
    }

    func update(_ data: ATypeOfData) {
        reset()
        let resource = ImageResource(downloadURL: url)
        backgroundImageView.kf.setImage(with: resource)
    }
}

The reason you are failing is you misunderstood reusing cell.

let updateCell = tableView.cellForRow(at: indexPath)

The updateCell can be a different cell with a cell which you want to use. It can be just a reused random cell. If you really want to track a cell, you need to set a property and check the property in cellForRowAt is same as the property in your image loading closure. I really recommend you to use the pattern I attached.

Ryan
  • 4,799
  • 1
  • 29
  • 56
  • Hey ryan thanks for providing a full example using kingfisher. I really appreciate it. I woudnt like to use 3rd party lib for this thought.. do you know how to do it manually ? If cant get it right my last resort would definitely using kingfisher – lorenzo gonzalez Oct 26 '17 at 00:10
  • Loading image and caching is not simple as you think. Also that is different topic. There are many good answers for image caching. – Ryan Oct 26 '17 at 03:34