Fixed assertion failure when reloading with animations

[geraldeersteling]

Hello,

Great work on the Kit, I really like the setup. I do encounter an assertion failure when trying to use reloading with animations.

I'm creating a viewcontroller with a tableview which includes search. The content (models) are supplied through an JSON (which is parsed to a correct ModelProtocol).
Everything goes fine if I use/reload the tableview without animations, but as soon as I use reloadData(after:onEnd:) my app crashes with an assertion failure.

Here is a snippet of my code:

override public func viewDidLoad() {
    super.viewDidLoad()

    director = tableView.director
    director?.rowHeight = .autoLayout(estimated: UITableViewAutomaticDimension)
    let adapter = TableAdapter<Contact, ContactCell>()
    director?.register(adapters: [adapter])
}

public func updateSearchResults(for searchController: UISearchController) {
    // Assume the models contain valid `Contact` objects retrieved from the JSON
    let models = retrieveContacts()
    DispatchQueue.main.async {
        self.director?.reloadData(after: {
            $0.removeAll()
            $0.add(section: TableSection(result?.resultCollection))
            return .default()
        })
    }
}

The above snippet triggers an assertion exception which tells:

Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of sections. The number of sections contained in the table view after the update (1) must be equal to the number of sections contained in the table view before the update (0), plus or minus the number of sections inserted or deleted (0 inserted, 0 deleted).'

This happens even if I remove the line $0.removeAll(). Somehow the deleteSections is not working properly.

Am I doing something wrong here?

Thanks

[ghost]

Same problem here. Any news ?

[geraldeersteling]

Nope, nothing heard from the owner yet. 😕

[wow-such-amazing]

@malcommac

Seems like the problem is inside this function.

public func reloadData(after task: ((TableDirector) -> (TableReloadAnimations?))? = nil, onEnd: (() -> (Void))? = nil)

In this lines

// Execute reload for sections
let changesInSection = SectionChanges.fromTableSections(old: oldSections, new: self.sections)
changesInSection.applyChanges(toTable: self.tableView, withAnimations: animationsToPerform)

Because inside applyChanges(toTable: , withAnimations: ) library tries to insert/remove/move sections with animations, but beginUpdates/endUpdates is not called.

If you move this line
changesInSection.applyChanges(toTable: self.tableView, withAnimations: animationsToPerform)
after self.tableView?.beginUpdates() then it will update the table.

I also think that it would be great to add if statement between iOS 11 and lower to use performBatchUpdates for tableView.

[geraldeersteling]

I can confirm that what @crabman448 suggested works; my initial table content was loaded with an animation. However; I encountered a subsequent crash after trying to add more models to a section:

Could not cast value of type 'MyFramework.SomeModel' (0x10067c290) to 'Swift.AnyHashable' (0x10208f3b0)

I've checked and it seems by conforming to ModelProtocol you're not conforming to Hashable because ModelProtocol itself is not directly Hashable and/or Equatable.
The protocol only has a where clause, i.e.:

ModelProtocol where Self: Equatable

For now I've manually implemented Hashable and Equatable for my model and it seems to work now...but these should be introduced as bugfixes I think.

[ghost]

@geraldeersteling Yes the documentation explicitly recommend to conform to both ModelProtocol and Hashable, by implementing var identifier: Int and the == operator overload :

public class Contact: ModelProtocol, Hashable {
	...

	public var identifier: Int {
		return GUID.hashValue
	}
	
	public static func == (lhs: Contact, rhs: Contact) -> Bool {
		return lhs.GUID == rhs.GUID
	}
}

[wow-such-amazing]

@geraldeersteling I think you should implement Hashable by your own.
There are couple places in the docs that states that you need to do this

A single model type (any class which is conform to ModelProtocol and Hashable) 
can be represented by one and only View (while a View can be used to represent different models).

Models array can be etherogenous, just remeber to make your objects conform to ModelProtocol 
and Hashable protocol and register the associated adapter. 
FlowKit will take care to call your adapter events as it needs.

And with swift 4 it is easy yo conform to Equatable or Hashable. Because the compiler can generate code if you specified that your model is Hashable or Equatable.

[geraldeersteling]

You're right @OlivierLapraye / @crabman448, I overlooked that part. I implemented the ModelProtocol but didn't took note of the docs saying Hashable should be implemented too.