Add AutoRegistering protocol for automatic cell registration

[apptekstudios]

We implemented auto-Registration of tableViewCell types. This was done such that it is an optional, non-breaking change. The features is disabled by default, although that could be changed without breaking anything.

When calling dequeueReusableCell(...), one can now specify autoRegister: true, and it will take care of ensuring that the class/nib is registered. Note that in order to get around generic constraints this required duplicating the dequeueReusableCell function for each of Reusable and NibReusable.

Hopefully this is useful to someone!

[AliSoftware]

Some nitpickings.

Also, I'm not sure about that generic constraints limitation you talked about that forced you to duplicate the code, can you elaborate about the error you get if you don't duplicate?

[apptekstudios]

The generic constraints issue relates to the dequeueReusableCell<T: UITableViewCell> function casting T as 'Reusable', meaning that the subsequent call to register(cellType:) tries to register the class rather than the nib (in cases where it is a NibReusable).

Swift doesn't seem to like conditionally casting to a protocol, so this was the simple workaround. There might be a non-duplicating way around this that I haven't thought of though!

[AliSoftware]

Looking good!

I think maybe we could still find a way to factorize the two same methods (which differ only from their where clause) to make them call a common private method.

After all the only difference is that each variant of this method calls a different variant of register(cellType:).

So if for example we provide a private helper method which takes a register: (() -> Void)? closure as a parameter (and no autoRegister: Bool), and do all the common stuff common to these two methods in there, then our two variants of the real public functions will just need to call that private helper function with registerClosure: autoRegister ? { register(cellType: cellType) } : nil — avoiding to duplicate the common code. Even if we'd still need to duplicate the methods, at least the implementations would be factorized that way

What do you think?

[AliSoftware]

FYI the CI failure is due to the following linter errors:

…
Linting 'MyCustomWidget.swift' (25/25)
/Users/distiller/project/Example/ReusableDemo iOS/TableViewController.swift:19: warning: Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
/Users/distiller/project/Example/ReusableDemo iOS/TableViewController.swift:66: warning: Line Length Violation: Line should be 120 characters or less: currently 146 characters (line_length)

[AliSoftware]

@apptekstudios I took the liberty to push a commit on your fork to show my idea of mutualising the code. (Also added missing documentation on the new method)

There are still things missing in that PR imho:

• You implemented this for UITableView.dequeueReusableCell, but why not for all the other stuff? Like dequeueHeaderFooter, and like for the similar methods in UICollectionViews as well?
• The more I think about this, the more I wonder if it wouldn't be nicer — both for API and for usage — to use additional protocol conformance instead of adding the autoRegister: Bool parameter (that you'll have to not forget to add at call site on every place you dequeue your cells otherwise)?

So maybe it would be more powerful, flexible, and less error-prone for the end user, to declare a protocol AutoRegistering {} for example in Reusable, and make different implementations for where T: NibReusable, T: AutoRegistering vs just where T: NibReusable (the compiler always choose the most specific implementation anyway)?

That way users of the Reusable library would just have to just add AutoRegistering to their custom UITableViewCell subclasses (the same way they just add Reusable or NibReusable or NibLoadable already), making auto-registration still an opt-in feature, but without having to think about explicitly adding autoRegister: true everywhere they want to dequeue such cells. This would also have the advantage for the lib user to not have to remember which cells are auto-registered or not so to which dequeue calls they have to add autoRegister: true vs not add it.

[AliSoftware]

I've just pushed a commit to show you the idea of the alternative solution. I hope you like it 😉

I'll let you take over to complete the PR:

• Implement the same idea for Headers/Footers
• Implement the same idea for UICollectionViews
• Update the documentation to explain the new AutoRegistering protocol

👍

[apptekstudios]

Thanks for implementing the protocol - I agree that it's definitely a much clearer solution.

I've implemented the AutoRegistering for Headers/Footers and added an example 👍. Hopefully I'll find time to write a quick documentation update tomorrow 🙂

[apptekstudios]

Regarding UICollectionView: it seems that there's no API we can utilise to determine whether a class has been registered yet or not.

A far less elegant solution would be to simply call register each time - although that would surely impact performance... (this depends on whether Apple's underlying implementation of the register(_:, forCellWithReuseIdentifier:) method would simply ignore a repeat call)

[AliSoftware]

Ah, bummer.

(I now remember why I'm personally not a fan of doing the auto registering dance in my own projects but prefer explicit registration 😅)

Ok maybe we could indeed register every time before dequeuing (but only for collection views then), but we'd need to clearly document that and the performance trade-off in the documentation to make it crystal clear to users of Reusable then.

Another way would be to keep our own list of registered cell classes per collection view (maybe using ObjC's associated objects?) so we can test before registration. But I wonder if that's not gonna be a similar performance impact anyway…

[apptekstudios]

I think associated objects is a good way to do it. I went ahead and implemented it 👍

Shouldn't have any performance impact, because all that is done is creating a single object per CollectionView which then tracks whether the cellType has been registered by us yet.

[AliSoftware]

Linting 'InfoViewController.swift' (17/26)
/Users/distiller/project/Example/ReusableDemo iOS/CollectionViewController.swift:19: warning: Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace)
/Users/distiller/project/Example/ReusableDemo iOS/CollectionViewController.swift:23: warning: Trailing Whitespace Violation: Lines should not have trailing whitespace. (trailing_whitespace

[Alex293]

Disclaimer: Did not checked the code so you may be already doing this.

In a project I was doing mostly the same thing. For checking if the cell is registered I was trying to dequeue. If it throw -> catch, register then dequeue again

[AliSoftware]

@Alex293 but doesn't it throw an ObjC NSException in those cases? Which can't be caught from Swift (*) and are different from Swift's thrown Errors?

---

(*) well, unless if you create an ObjC helper method which catches the NSException from ObjC then converts it to an NSError returned by reference which is interpreted as a thrown Error in Swift… some ObjC pods provide such a helper but I wouldn't want Reusable to depend on another pod just for that — nor to implement that in Reusable itself, requiring to add ObjC to my pod and a bridging header and all that just for that…

[AliSoftware]

@apptekstudios What do you think about the alternate solution of catching the ObjC exception mentioned above (instead of going the whole way of using associated objects)?

Something like:

// ObjCException.h
#import <Foundation/Foundation.h>
@interface ObjC : NSObject
+ (BOOL)catch:(void(^)(void))tryBlock error:(__autoreleasing NSError **)error;
@end

// ObjCException.m
#import "ObjCException.h"
@implementation ObjCException
+ (BOOL)catch:(void(^)(void))tryBlock error:(__autoreleasing NSError **)error {
    @try {
        tryBlock();
        return YES;
    }
    @catch (NSException *exception) {
        *error = [[NSError alloc] initWithDomain:exception.name code:0 userInfo:exception.userInfo];
        return NO;
    }
}
@end

Then using it like this from Swift:

let cell = try? ObjCException.catch {
  return collectionView.dequeue(…)
}

I'm not sure which one is best, as:

• Catching the ObjC exception would allow us to avoid having to add so much code with associated objects, and will probably simplify the code/implementation a lot (and make the code for CollectionViewCells a lot more similar to the one for TableViewCells too)
• Using associated objects maybe adds a performance toll I think? really not much, but still a AO lookup + dictionary lookup… maybe I'm overthinking this and that's not even noticeable, but I wouldn't want people using Reusable having a performance hit and deciding to drop Reusable because of it
• Adding the "catch-ObjC-exception" code means adding a little ObjC to the code. It's been a long time since I mixed ObjC with Swift, I'm not sure if there would be any impact on CocoaPods level, like having to declare a bridging header manually or whatnot. I don't think there's much drawback but we might want to check anyway.

What do you think?

[apptekstudios]

I had a go at catching the objective C exceptions, however couldn't get anything to compile properly with the objective C included in the pod.
https://github.com/apptekstudios/Reusable/tree/objcExceptions

My opinion definitely leans toward keeping the library swift-only. It's a pity apple hasn't yet bridged ObjC-exceptions in UIKit methods across to swift errors...

[AliSoftware]

All right then. Could you just address the nitpickings I made in my latest review? I'll look into merging that next.

[apptekstudios]

Sorry for taking so long to get back to this - I've fixed those minor issues, pulled all changes from your master repo, and updated the changelog.

[AliSoftware]

Shouldn't we pass for: indexPath in that call here?