How to properly manage elaborate Collection View cellForItemAt method?

Question

I'm a fairly new developer and I have some long cellForItemAt methods. I have a feeling I'm missing something important.

In this ViewController I have a segmented control that filters data with a boolean taskView property.

Here's what my cellForItemAt call looks like:

func collectionView(_ collectionView: UICollectionView, cellForItemAt indexPath: IndexPath) -> UICollectionViewCell {
        
        let cell = collectionView.dequeueReusableCell(withReuseIdentifier: TaskCell.reuseIdentifier, for: indexPath) as! TaskCell
        
        //SwipeCell Delegate
        cell.delegate = self
        
        var transactionResults: Results<Transaction>
        
        if taskView {
            transactionResults = unclearedTransactionsToDate
        } else {
            transactionResults = allTransactions
        }
        
        cell.configureCollectionViewCells(indexPath, transactionResults)
        
        let balanceAtDate: Double = realm.objects(Transaction.self).filter("transactionDate <= %@", transactionResults[indexPath.item].transactionDate).sum(ofProperty: "transactionAmount")
        
        cell.balanceLabel.attributedText = balanceAtDate.toAttributedString(size: 9, offset: 6)
        
        if transactionResults[indexPath.item].isCleared == false && !taskView {
            cell.amountLabel.textColor = .lightGray
            cell.subcategoryLabel.textColor = .lightGray
            cell.dateLabel.textColor = .lightGray
            cell.balanceLabel.textColor = .lightGray
            cell.circleView.backgroundColor = .lightGray
        } else {
            cell.subcategoryLabel.textColor = .black
            cell.dateLabel.textColor = .black
            cell.balanceLabel.textColor = .black
            cell.circleView.backgroundColor = UIColor(rgb: transactionResults[indexPath.item].transactionCategory!.categoryColor)
        }
        
        return cell
    }
}

And in my configureCollectionViewCells method I have:

func configureCollectionViewCells(_ indexPath: IndexPath, _ transaction: Results<Transaction>) {
        
        imageView.image = UIImage(named: transaction[indexPath.item].transactionCategory!.categoryName)
        imageView.tintColor = .white
        circleView.backgroundColor = UIColor(rgb: transaction[indexPath.item].transactionCategory!.categoryColor)
        subcategoryLabel.textColor = .black
        dateLabel.textColor = .black
        balanceLabel.textColor = .black
        subcategoryLabel.text = transaction[indexPath.item].transactionSubCategory?.subCategoryName
        amountLabel.attributedText = transaction[indexPath.item].transactionAmount.toAttributedString(size: 9, offset: 6)
        
        
        
        let formatter = DateFormatter()
        
        formatter.dateFormat = "MMMM dd, yyyy"
        
        let dateString = formatter.string(from: transaction[indexPath.item].transactionDate)
        
        dateLabel.text = dateString
        
        if transaction[indexPath.item].transactionAmount > 0 {
            
            amountLabel.textColor = UIColor(rgb: Constants.green)
            
        } else {
            
            amountLabel.textColor = UIColor(rgb: Constants.red)
            
        }
    }

The code works, but I have a feeling that it's not being implemented properly. Can someone give me some advice (keeping in mind that I'm new to programming) on how to manage such lengthy functions? I feel like I'm missing a couple of concepts.

Some people have just said "Throw everything in cellForItemAt" and some only have 3 lines in cellForItemAt.

I think maybe I should be overriding the layoutSubviews method in the collectionView cell and implementing some of the code there.

Any general or specific advice is greatly appreciated. I would also be interested in researching if anyone has any resources on the topic.

Thank you in advance.

The accepted answer is spot on. To cut a bit more to the chase for your specific code, don't do disk or network operations during UI updates. Do those before, ensuring your dataSource is populated and then update the UI. e.g. don't do this `let balanceAtDate: Double = realm.objects(Transaction.self)` inside any UI calls as if that's happening for every cell, it could cause the user experience to be laggy or inconsistent. That operation should be more of a C)ontroller operation in [MVC](https://en.wikipedia.org/wiki/Model–view–controller) models. — Jay, Oct 04 '20 at 14:29
@Jay, thank you for the advice. How do I do this? Would I create a computed property instead? Or create a function to filter and sum with an IndexPath input parameter? Or create a model that calculates the balance? I can't wrap my brain around this. — BriScoLeg, Oct 05 '20 at 00:51
While you could technically do something within a computed property, that's probably not ideal. Using MVC (Read the above link), your controller would control your data - loading it, saving it, sorting it, etc. Your controller would interface with the models that contain that data as well, and the View would display the model data. Generally speaking when your ViewController starts up, it would load data and populate models that are stored in the dataSource (often an array) and the views (tableView for example) would display the data contained in those models. — Jay, Oct 05 '20 at 18:54

score 3 · Accepted Answer · answered Oct 03 '20 at 22:08

What you're doing is not "wrong" in and of itself. However, there is a school of thought which says that, ideally, cellForRowAt should know nothing of the internal interface of the cell. This (cellForRowAt) is the data source. It should hand the cell just the data. You have a cell subclass (TaskCell), so it just needs some methods or properties that allow it to be told what the data is, and the cell should then format itself and populate its own interface in accordance with those settings.

If all of that formatting and configuration code is moved into the cell subclass, the cellForRowAt implementation will be much shorter, cleaner, and clearer, and the division of labor will be more appropriate.

_{In support of this philosophy, I would just add that Apple has adopted it in iOS 14, where a cell can now have a UIContentConfiguration object whose job is to communicate the data from cellForRowAt into the contentView of the cell. So for example instead of saying (for a table view cell) cell.textLabel.text = "howdy" you say configuration.text = "howdy" and let the configuration object worry about the fact that a UILabel might be involved in the interface.}

How to properly manage elaborate Collection View cellForItemAt method?

1 Answers1