NibReusable -> Reusable & NibLoadable

[nekrich]

Hi.

There is no need in strict NibReusable protocol conforming in register functions. You can use protocol composition type.

[AliSoftware]

Thanks for this PR!

Yup, that convenience NibReusable protocol was added for convenience both for the consumer (easier to make your tableView cells conform to a single protocol) and clarity when reading the source.
But that was before Swift 3, when the syntax was still protocol<Reusable, NibLoadable> which was a bit verbose to use. Now that the syntax has changed in Swift 3 that convenience protocol isn't really needed anymore indeed 😉

[AliSoftware]

Could you just please:

• Update the README file accordingly (some documentation references that convenience protocol so if we get rid of it we should update the doc too)
• Add an entry in the CHANGELOG to mention the change and credit yourself

Thanks!

[nekrich]

I think that you should leave NibReusable as syntax sugar.

[AliSoftware]

I think that you should leave NibReusable as syntax sugar.

🤔 But then what's the point of this PR, if you don't mind me asking?

[nekrich]

If you will make a subclass of Reusable cell and make it NibLoadable you will get error on register of subclass. Because it doesn't conforms to NibReusable, but if you try to conform it to NibReusable you will get error that your subclass conforms to Reusable protocol twice.

Or you just make a cell that is Reusable, NibLoadable and you will get an error too.

That is the point of PR.

Lets create cell with theme.

class MyCellWithTheme: UITableViewCell, Reusable {
  var theme: Theme {
    didSet {
      setupTheme()
    }
  }
  func setupTheme() {
     // set bg color, fonts for default labels
  }
}

And this will not work in 3.0.0, but succeeds in my PR.

// Subclass
class MyCustomCellWithTheme: MyCellWithTheme, NibLoadable {
  @IBOutlet private var doStuffbutton: UIButton!
  override func setupTheme() {
    super.setTheme(theme)
     // style for button
  }
}

// Two protocols conforming
class MyAnotherCell: UITableViewCell, Reusable, NibLoadable {
}

And it will not brake existing API, so there is no need to deprecate NibLoadable protocol, and now it can be used as syntax sugar (no strict conforming).

// And this will work too
class MyAnotherCell: UITableViewCell, NibReusable {
}

[AliSoftware]

Ok, I think I understand, but what surprises me then is that doing this change:

-  final func register<T: UICollectionViewCell>(cellType: T.Type) where T: NibReusable {
+  final func register<T: UICollectionViewCell>(cellType: T.Type) where T: Reusable & NibLoadable {

Would change anything in your scenario… I mean, isn't it the same to conform to NibReusable (which is convenience for Reusable & NibLoadable) or to conform to Reusable & NibLoadable?

Or maybe if it is different — because NibReusable is a separate protocol conforming to both protocols and not strictly a & protocol composition — it would be simpler if that PR just changed protocol NibReusable: Reusable, NibLoadable to typealias NibReusable = Reusable & NibLoadable?

[nekrich]

Yep, it is different — because NibReusable is a separate protocol.

😱 typealias for protocol composition it's brilliant idea.

Reusable & NibLoadable equals to NibReusable in both cases
a)typealias NibReusable = Reusable & NibLoadable. Reverse equatable.
b) protocol NibReusable: Reusable, NibLoadable. Reusable, NibLoadable object can't conform to NibReusable.

But I think conformance to 2 protocols Reusable & NibLoadable in func definitions is better than conformance to one typealias NibReusable it clearly shows a list of base protocols to which you should conform.
Cause at first look at this function (as a new programmer who uses this framework at the first time) you will see, that you need to implement 2 protocols in your class.

final func register<T: UICollectionViewCell>(cellType: T.Type) where T: Reusable & NibLoadable

And in the case below you will see one protocol, but in fact there are two of them, so you should check both of them anyway, but it implicit.

final func register<T: UICollectionViewCell>(cellType: T.Type) where T: NibReusable

I'm agree that those protocols conform-and-go, but I prefer to check all frameworks used by me (especially small ones) for what is under the hood.

[AliSoftware]

Cool, let's go for this then! 👍

• Using typealias NibReusable instead of protocol NibReusable, but still keeping it (both for retro-compatibility and convenience for users of the lib)
• Switching to explicit use of Reusable & NibLoadable in the function implementations is ok for me if you think it's more clear

[AliSoftware]

Now that we've decided to keep NibReusable as a typealias, maybe keep the usage of NibReusable in the example project here, especially since it's already explained in the comment that it's in fact just a convenience name combining both.

btw, you might also want to check the doc-comments throughout the code and sample project where we describe NibReusable as "just a convenience protocol that combines both NibLoadable + Reusable protocols" and change it to say it's a "convenience typealias combining NibLoadable & Reusable" instead.

[AliSoftware]

Please add 2 spaces after your full-stop here (see other entries for inspiration).
Ending a line with two spaces in Markdown allows to go to the next line in the same paragraph (like a <br>) in the rendered document.

---

The typical formatting I use in the CHANGELOG is this, where • represent a space:

* Change description.••
  [@your_github_handle](link_to_your_gh_profile)
  [#PR_Num](link_to_this_pr)

[AliSoftware]

Use the NibReusable typealias

[AliSoftware]

I found it kinda nice to add the information/aka you added before, like Use the NibReusable typealias (equivalent to Reusable & NibLoadable) if …

[AliSoftware]

🤔 That's the 4th time I relaunch the Travis-CI build because each time it errors (it doesn't launch the job and fail, no, it just errors without even launching the build).

I have no idea if that's linked to the shortage that Travis-CI is currently experiencing with OSX jobs or not, so if we should change something to make it build again or not, I'm a bit confused…

(same problem with #38)

[AliSoftware]

Ok, still don't know why the Travis job keeps erroring, and I have no idea how to investigate further… so merging anyway after testing it locally.