Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How should I refactor my custom UITableView to improve maintainability

I've got a UITableView with many different kind of views. In each method of the UITableView data source I need to check the type of the cell and type of the object, cast them, and act correctly. This is not very clean (it works) but not very maintainable.

So I was working on something to abstract this part but I'm a little bit stuck. The following code is simplified and maybe not that useful but it is to demonstrate my current problem:

extension UITableView {
    func dequeue<T: UITableViewCell>(_ type: T.Type,
                                     for indexPath: IndexPath) -> T {
        let cell = dequeueReusableCell(withIdentifier: String(describing: type),
                                       for: indexPath)
        guard let cellT = cell as? T else {
            fatalError("Dequeue failed, expect: \(type) was: \(cell)")
        }

        return cellT
    }
}

struct Row<Model, Cell> {
    let view: Cell.Type
    let model: Model

    var fill: ((Model, Cell) -> Void)
}

// Completly unrelated models
struct Person {
    let name: String
}

struct Animal {
    let age: Int
}

// Completely unrelated views
class PersonView: UITableViewCell {

}

class AnimalView: UITableViewCell {

}


// Usage:
let person = Person(name: "Haagenti")
let animal = Animal(age: 12)

let personRow = Row(view: PersonView.self, model: person) { person, cell in
    print(person.name)
}

let animalRow = Row(view: AnimalView.self, model: animal) { animal, cell in
    print(animal.age)
}

let rows = [
//    personRow
    animalRow
]



let tableView = UITableView()
for row in rows {
    tableView.register(row.view, forCellReuseIdentifier: String(describing: row.view))


    let indexPath = IndexPath(row: 0, section: 0)
    let cell = tableView.dequeue(row.view, for: indexPath)

    row.fill(row.model, cell)
}

The code works, but when I enable the animalRow Swift will complain. This is not that surprising since it cannot resolve the types. I cannot figure out how to get around this.

By using the following code I can declare everything once and execute all the parts like "fill" when I need them. I will also add code like onTap etc, but I removed all this code to keep to problem clear.

like image 402
Haagenti Avatar asked Feb 11 '19 12:02

Haagenti


1 Answers

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.

like image 58
farzadshbfn Avatar answered Nov 14 '22 21:11

farzadshbfn