0

I have a UItableViewCell with a button inside it, I set the tag of the button and add the action of the button in my ViewController using the tag.

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let cell = tableView.dequeueReusableCell(withIdentifier: "BillHistoryTableViewCell", for: indexPath) as! BillHistoryTableViewCell
        let cellData = billHistories[indexPath.row]
        cell.setup(with: cellData)
        cell.retryButton.tag = indexPath.row
        return cell
}

@IBAction func billHistoryRetryButtonDidTap(_ sender: UIButton) {
        let index = sender.tag
        if let id = billHistories[index].transactionInfo?.billUniqueID {
            hidePayIdGeneralTextField()
            billIdTextField.text = id.toNormalNumber()
            inquiryGeneralBillRequest()
        }
}

I want to know is it wrong for any reason? someone told me it is not good because it uses lots of memory to use tags.

Alireza12t
  • 377
  • 1
  • 4
  • 14
  • 1
    This might work fine if the tableview is used just for display purposes without any changes. I don't think memory is the biggest concern in my opinion. Imaging a case where the cell is removed or reordered without the tableview not being reloaded - you will face issues by retrieving the wrong data and also could be a potential for an out of bounds crash. – Shawn Frank Jan 19 '22 at 17:06
  • It isn't a good idea for the reasons Shawn outlined. There are [better ways](https://stackoverflow.com/questions/28659845/how-to-get-the-indexpath-row-when-an-element-is-activated/38941510) – Paulw11 Jan 19 '22 at 18:36
  • @ShawnFrank I do very complex views based on collection/table views and tags with buttons. If you reorganise cells then it's just a case of updating visible cells. One simple function call with short simple loop inside. Just after every reordering/deleting/inserting/etc. No big deal. – Ariel Bogdziewicz Jan 19 '22 at 21:53
  • 1
    I agree with you @ArielBogdziewicz - my main point is you need to identify that you need to handle keeping the tags up to date yourself. There might be solutions which are easier to implement. That's all I meant. – Shawn Frank Jan 20 '22 at 03:24

1 Answers1

1

Will it work? yes, but as mentioned above, this is not the best approach, I'd avoid using tags unless this is just for some POC. There are better approaches to handle it. The first I'd suggest is using delegation to inform back to the controller, here's an example:

class BillHistoryTableViewController {
    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let cell = tableView.dequeueReusableCell(withIdentifier: "BillHistoryTableViewCell", for: indexPath) as! BillHistoryTableViewCell
        let cellData = billHistories[indexPath.row]
        cell.setup(with: cellData)
        cell.index = indexPath.row
        
        cell.delegate = self
        return cell
    }
}

extension BillHistoryTableViewController: BillHistoryTableViewCellDelegate {
    func didTapButton(index: Int) {
        print("tapped cell with index:\(index)")

        if let id = billHistories[index].transactionInfo?.billUniqueID {
            hidePayIdGeneralTextField()
            billIdTextField.text = id.toNormalNumber()
            inquiryGeneralBillRequest()
        }
    }
}

protocol BillHistoryTableViewCellDelegate: AnyObject {
    func didTapButton(index: Int)
}

class BillHistoryTableViewCell: UITableViewCell {
    weak var delegate: BillHistoryTableViewCellDelegate?
    var cellData: CellData?
    var index: Int?
    
    func setup(with cellData: CellData) {
        self.cellData = cellData
    }
    
    @IBAction func buttonPressed(_ sender: UIButton) {
        guard let index = index else {
            return
        }
        
        delegate?.didTapButton(index: index)
    }
}

Another approach that I prefer lately is using Combine's PassThroughSubject, it requires less wiring and delegate definitions.

import Combine

class BillHistoryTableViewController {
    var cancellable: AnyCancellable?
    
    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let cell = tableView.dequeueReusableCell(withIdentifier: "BillHistoryTableViewCell", for: indexPath) as! BillHistoryTableViewCell
        let cellData = billHistories[indexPath.row]
        cell.setup(with: cellData)
        cell.index = indexPath.row
        
        cancellable = cell.tappedButtonSubject.sink { [weak self] index in
            guard let self = self else { return }
            print("tapped cell with index:\(index)")

            if let id = self.billHistories[index].transactionInfo?.billUniqueID {
                self.hidePayIdGeneralTextField()
                self.billIdTextField.text = id.toNormalNumber()
                self.inquiryGeneralBillRequest()
            }
        }
        
        return cell
    }
}

class BillHistoryTableViewCell: UITableViewCell {
    var tappedButtonSubject = PassthroughSubject<Int, Never>()
    
    var cellData: CellData?
    var index: Int?
    
    func setup(with cellData: CellData) {
        self.cellData = cellData
    }
    
    @IBAction func buttonPressed(_ sender: UIButton) {
        guard let index = index else {
            return
        }
        
        tappedButtonSubject.send(index)
    }
}

You can make it even shorter by injecting the index with the cellData, e.g:

func setup(with cellData: CellData, index: Int) {
    self.cellData = cellData
    self.index = index
}

but from what I see in your example, you don't even need the index, you just need the CellData, so if we'll take the Combine examples these are the main small changes you'll have to make:

var tappedButtonSubject = PassthroughSubject<CellData, Never>()

tappedButtonSubject.send(cellData)

and observing it by:

cancellable = cell.tappedButtonSubject.sink { [weak self] cellData in
    if let id = cellData.transactionInfo?.billUniqueID {
        //
    }
}
Maor Atlas
  • 107
  • 5