How should I refactor my custom UITableView to improve maintainability

Sahil Manchanda's answer is covering the OOD approach to solving this problem but as a drawback you have to define your models as class.

First thing we need to consider is the fact that we're discussing about maintainability here, so in my humble opinion, Model should not know about the view (or which views it's compatible with), That is Controller's responsibility. (what if we want to use the same Model for another view somewhere else?)

Second thing is that if we want to abstract it to higher levels, it will definitely require down-cast/force-cast at some point, so there is a trade-off to how much it can be abstracted.

So for sake of maintainability, we can increase the readability and separation of concern/local reasoning.

I suggest to use an enum with associatedValue for your models:

enum Row {
    case animal(Animal)
    case person(Person)
}

Well right now our Models are separated and we can act differently based on them.

Now we have to come-up with a solution for Cells, I usually use this protocol in my code:

protocol ModelFillible where Self: UIView {
    associatedtype Model

    func fill(with model: Model)
}

extension ModelFillible {
    func filled(with model: Model) -> Self {
        self.fill(with: model)
        return self
    }
}

So, we can make our cells conform to ModelFillible:

extension PersonCell: ModelFillible {
    typealias Model = Person

    func fill(with model: Person) { /* customize cell with person */ }
}

extension AnimalCell: ModelFillible {
    typealias Model = Animal

    func fill(with model: Animal) { /* customize cell with animal */ }
}

Right now we have to glue them all together. We can refactor our delegate method tableView(_, cellForRow:_) just like this:

var rows: [Row] = [.person(Person()), .animal(Animal())]

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    switch rows[indexPath.row] {
    case .person(let person): return (tableView.dequeue(for: indexPath) as PersonCell).filled(with: person)
    case .animal(let animal): return (tableView.dequeue(for: indexPath) as AnimalCell).filled(with: animal)
    }
}

I believe in future this is more readable/maintainable than down-casting in Views or Models.

Suggestion

I also suggest to decouple PersonCell from Person too, and use it like this:

extension PersonCell: ModelFillible {
    struct Model {
        let title: String
    }

    func fill(with model: Model { /* customize cell with model.title */ }
}

extension PersonCell.Model {
    init(_ person: Person) { /* generate title from person */ }
}

And in your tableView delegate use it like this:

func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
    switch rows[indexPath.row] {
    case .person(let person): return (tableView.dequeue(for: indexPath) as PersonCell).filled(with: .init(person))
    case .animal(let animal): return (tableView.dequeue(for: indexPath) as AnimalCell).filled(with: .init(animal))
    }
}

With current approach compiler will always know what's going on, and will block you from making mistakes & in future by reading this code, you know exactly what's going on.

Note

The reason that it will require down-cast/force-cast at some point if we try to abstract it to higher levels (just like Sahil's answer), is the fact that dequeue does not happen at the same-time we want to fill/customize our cell. dequeue has to return a type known to compiler. it's either UITableViewCell, PersonCell or AnimalCell. In first case we have to down-cast it, and it's not possible to abstract PersonCell and AnimalCell (unless we try down-cast/force-cast in their models). We can use a type like GenericCell<Row> and also cell.fill(with: row) but that means that our customized cell, has to handle all cases internally (it should handle PersonCell and AnimalCell views at the same time which is also not maintainable).

Without down-cast/force-cast this is the best I got to over the years. If you need more abstractions (single line for dequeue, and a single line for fill) Sahil's answer is the best way to go.

Have a look at the following struct:

protocol MyDelegate {
    func yourDelegateFunctionForPerson(model: Person)
    func yourDelegateFunctionForAnimal(model: Animal)
}


enum CellTypes: String{
    case person = "personCell"
    case animal = "animalCell"
}

Base Model

class BaseModel{
    var type: CellTypes

    init(type: CellTypes) {
        self.type = type
    }
}

Person Model

class Person: BaseModel{
    var name: String
    init(name: String, type: CellTypes) {
        self.name = name
        super.init(type: type)
    }
}

Animal Model

class Animal: BaseModel{
    var weight: String
    init(weight: String, type: CellTypes) {
        self.weight = weight
        super.init(type: type)
    }
}

Base Cell

class BaseCell: UITableViewCell{
    var model: BaseModel?
}

Person Cell

class PersonCell: BaseCell{
    override var model: BaseModel?{
        didSet{
            guard let model = model as? Person else {fatalError("Wrong Model")}
            // do what ever you want with this Person Instance
        }
    }
}

Animal Cell

class AnimalCell: BaseCell{
    override var model: BaseModel?{
        didSet{
            guard let model = model as? Animal else {fatalError("Wrong Model")}
            // do what ever you want with this Animal Instance
        }
    }
}

View Controller

    class ViewController: UIViewController{
    @IBOutlet weak var tableView: UITableView!

    var list = [BaseModel]()

    override func viewDidLoad() {
        super.viewDidLoad()
        setupList()
    }

    func setupList(){
        let person = Person(name: "John Doe", type: .person)
        let animal = Animal(weight: "80 KG", type: .animal)
        list.append(person)
        list.append(animal)
        tableView.dataSource = self
    }
}

extension ViewController: UITableViewDataSource{

    func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
        let model = list[indexPath.row]
        let cell = tableView.dequeueReusableCell(withIdentifier: model.type.rawValue, for: indexPath) as! BaseCell
        cell.model = model
        cell.delegate = self
        return cell
    }

    func tableView(_ tableView: UITableView, numberOfRowsInSection section: Int) -> Int {
            return list.count
    }
}

extension ViewController: MyDelegate{
    func yourDelegateFunctionForPerson(model: Person) {

    }

    func yourDelegateFunctionForAnimal(model: Person) {

    }


}

MyDelegate protocol is used to perform "Tap" actions CellTypes enums is used to identify Cell Type and for dequeuing All of the Model class will inherit BaseModel which is quite useful and will eliminate the need to typecase in cellForRow at function. and All the tableViewCells have inherited BaseCell which holds two variables i.e. model and delegate. these are overridden in Person and Animal Cell.

Edit: Risk of losing Type Safety can certainly be reduced if you specify the 'celltype' directly in super.init() in model class. e.g.

class Person: BaseModel{
    var name: String
    init(name: String) {
        self.name = name
        super.init(type: .person)
    }
}

As cells are being dequeued with 'type' variable.. correct model will be supplied to correct cell.