Table View Data is overridden

Question

I have a UITableView. Its cell contains a label that will display a question, a yes button and a no button. The goal is to view questions one by one. First I call the API to get the questions in the viewDidLoad method:

override func viewDidLoad() {
        super.viewDidLoad()
        tableView.allowsSelection = false

        getQuestions(baseComplainID: "1") { (questions, error) in
            self.questions = questions
            DispatchQueue.main.async {
                self.tableView.reloadData()
            }
        }
    }

In the cellForRowAt method I display them one by one:

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        guard let cell = tableView.dequeueReusableCell(withIdentifier: "cell", for: indexPath) as? TableViewCell else {
            fatalError("Fatal Error")
        }
        cell.yesButton.isHidden = false
        cell.noButton.isHidden = false

        if indexPath.row + 1 == displayNumber {
            cell.questionLabel.text = questions[indexPath.row].question_name
        } else {
            cell.yesButton.isHidden = true
            cell.noButton.isHidden = true
        }

        cell.yesButton.addTarget(self, action: #selector(action), for: .touchUpInside)
        cell.noButton.addTarget(self, action: #selector(action), for: .touchUpInside)

        return cell
    }

and this is the action being executed on clicking yes or no:

@objc func action(sender: UIButton){
        let indexPath = self.tableView.indexPathForRow(at: sender.convert(CGPoint.zero, to: self.tableView))
        let cell = tableView.cellForRow(at: indexPath!) as? TableViewCell
        cell?.yesButton.isEnabled = false
        cell?.noButton.isEnabled = false

        if sender == cell?.yesButton {
            sender.setTitleColor(.black, for: .normal)
            sender.backgroundColor = .green
        } else {
            sender.setTitleColor(.black, for: .normal)
            sender.backgroundColor = .green
        }

        displayNumber += 1
        self.tableView.reloadData()
    }

Here I just change the background color of the button and increment the display number to display the next question.

All of this works perfect EXCEPT when I scroll, the data gets overridden and sometimes I find the question label empty and the questions replaces each other. I know this is normal due to the cell reusability but I don't know how to fix it.

Any suggestions please?

Answer 1

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        guard let cell = tableView.dequeueReusableCell(withIdentifier: "cell", for: indexPath) as? TableViewCell else {
            fatalError("Fatal Error")
        }
        cell.yesButton.isHidden = false
        cell.noButton.isHidden = false

        if indexPath.row + 1 == displayNumber {
            cell.questionLabel.text = questions[indexPath.row].question_name
        } else {
            cell.yesButton.isHidden = true
            cell.noButton.isHidden = true
        }

        cell.yesButton.addTarget(self, action: #selector(action), for: .touchUpInside)
        cell.noButton.addTarget(self, action: #selector(action), for: .touchUpInside)

        return cell
    }

i feel like your issue lies here in cellForRowAt function.

you have this written

if indexPath.row + 1 == displayNumber { your code here }

but i am unsure as to why you need this.

you should be doing something like this inside cellForRowAt

let data = self.questions
data = data[indexPath.row]
cell.questionLabel.text = data.question_name

you should not be adding 1 to your indexPath.row

Answer 2

You're going to need to keep track of your yes's no's and neither's for each cell. I'd tack an enum onto another data structure along with your questions. Your primary problem was that you were only keeping track of your question. You need to keep track of your answer as well. That way, when you load a cell, you can configure each button with the colors that you want in cellForRow(at:)

struct QuestionAndAnswer {
    enum Answer {
        case yes
        case no
        case nada
    }

    var question: Question
    var answer: Answer
}

And try not to reload your whole tableView when a button is pressed. tableView.reloadData() is expensive and distracting to the user. You should only be reloading the row that changed when a button was pressed.

Add callbacks on your cell so that you know which cell the corresponding buttons belong to. Notice how in the onYes and onNo callbacks we keep track of your "yes" or "no" selection then immediately reload the row below. When the row is reloaded, we finally know which color to make the button.

class AnswerCell: UITableViewCell {
    @IBOutlet weak var yesButton: UIButton!
    @IBOutlet weak var noButton: UIButton!

    var onYes: (() -> Void)) = {}
    var onNo: (() -> Void)) = {}
}

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

    // ...

    cell.yesButton.backgroundColor = qAndA.answer == .yes ? .green : .white
    cell.noButton.backgroundColor = qAndA.answer == .no ? .green : .white

    cell.onYes = {
        questionsAndAnswers[indexPath.row].answer = .yes
        tableView.reloadRows(at: [indexPath], with: .fade)
    }
    cell.onNo = {
        questionsAndAnswers[indexPath.row].answer = .no
        tableView.reloadRows(at: [indexPath], with: .fade)
    }

    // ...
}

Answer 3

Well, assume you have 10 questions, so a very simple and workaround fix is to declare a new array which has 10 elements as follow

var questionIsLoaded = Array(repeating:true , count 10)

the previous line will declare an array with 10 elements each element is bool which in our case will be true

then declare a function that handles if the question is loaded or not as follows, so if the question is loaded thus, the question with its indexPath should be marked as true and as a result, the yes and no buttons should be hidden else, the buttons should be shown

func handleQuestionIfLoaded(cell:yourCellType, indexPath:IndexPath) {
if questionIsLoaded[indexPath.row] , indexPath.row + 1 == displayNumber { {
questionIsLoaded[indexPath.row] = false
            cell.questionLabel.text = questions[indexPath.row].question_name
            cell.yesButton.isHidden = questionIsLoaded[indexPath.row]
            cell.noButton.isHidden = questionIsLoaded[indexPath.row]
        } else {
            cell.yesButton.isHidden = questionIsLoaded[indexPath.row]
            cell.noButton.isHidden = questionIsLoaded[indexPath.row]
        }

        cell.yesButton.addTarget(self, action: #selector(action), for: .touchUpInside)
        cell.noButton.addTarget(self, action: #selector(action), for: .touchUpInside)
}

then replace the body of cellForRowAt with the function above, then your action function will be as follows

@objc func action(sender: UIButton){
        let indexPath = self.tableView.indexPathForRow(at: sender.convert(CGPoint.zero, to: self.tableView))
        let cell = tableView.cellForRow(at: indexPath!) as? TableViewCell
        cell?.yesButton.isEnabled = questionIsLoaded[indexPath.row]
        cell?.noButton.isEnabled = questionIsLoaded[indexPath.row]

    if sender == cell?.yesButton {
        sender.setTitleColor(.black, for: .normal)
        sender.backgroundColor = .green
    } else {
        sender.setTitleColor(.black, for: .normal)
        sender.backgroundColor = .green
    }

    displayNumber += 1
    self.tableView.reloadData()
}

Now, your cells depend on an external dependency which is the array you have declared earlier, this means that when the cells are dequeued, they will be reused according to if the question is loaded or not by asking the array's element at the specific indexPath at first if the element is true or false