Delegate methods about ios-foundation HOT 11 OPEN

Really interesting idea! 🤔 But I agree with @vadymmarkov, it does make it a bit hard to find the API that you're looking for. It also reduces API discoverability and makes it harder to know where a method "comes from" when implemented. Finally, it would make our code not inline with Apple's naming conventions (if this is something that's important to us, which at least for Open Source, I personally think it should be) 😅

The nice thing about always using the class name as a prefix for the delegate method names is that things are crystal clear. There are also ways to make the APIs nicer without removing the prefix naming. In the example given in the first post func spotDidSelectItem(_ spot: Spotable, item: Item), we could make it nicer (and more "swifty"), by changing it to this:

func spot(_ spot: Spotable, itemSelected item: Item)

What do you guys think about that? 😄

from ios-foundation.

It will take time to get used to it, but I like the idea.

from ios-foundation.

This reads better. But I think you mean

func didSelect(item: Item, in spot: Spotable)

from ios-foundation.

@onmyway133 correct :) I'll fix it!

from ios-foundation.

Not sure about barcode scanner for example. Now we have this:

public protocol BarcodeScannerCodeDelegate: class {
  func barcodeScanner(_ controller: BarcodeScannerController, didCaptureCode code: String, type: String)
}

public protocol BarcodeScannerDismissalDelegate: class {
  func barcodeScannerDidDismiss(_ controller: BarcodeScannerController)
}

Should it be changed to:

public protocol BarcodeScannerCodeDelegate: class {
  func didCapture(code: String, type: String, in controller: BarcodeScannerController)
}

public protocol BarcodeScannerDismissalDelegate: class {
  func didDismiss(controller: BarcodeScannerController)
}

func didDismiss(controller: BarcodeScannerController) could cause conflicts:

from ios-foundation.

Oh I see, can you change the label to call it something else than controller would that help?

public protocol BarcodeScannerDismissalDelegate: class {
  func didDismiss(barcodeScanner: BarcodeScannerController)
}

from ios-foundation.

@zenangst It will solve the problem, but I still find this naming a bit confusing. For example:

// Old way:

protocol AdminHeaderViewDelegate: class {
  func adminHeaderViewDidExpand(_ adminHeaderView: AdminHeaderView)
}

delegate?.adminHeaderViewDidExpand(self)

// New way, option 1:

protocol AdminHeaderViewDelegate: class {
  func didExpand(adminHeaderView: AdminHeaderView)
}

delegate?.didExpand(adminHeaderView: self)

// New way, option 2
// Those 2 delegate methods make more sense, but are conflicting for compiler:

protocol AdminHeaderViewDelegate: class {
  func didExpand(in adminHeaderView: AdminHeaderView)
}

protocol AdminFooterViewDelegate: class {
  func didExpand(in adminFooterView: AdminFooterView)
}

Also when I type did in the controller I get a lot of methods not really related to delegates or to my AdminHeaderViewDelegate specifically:

from ios-foundation.

I like New way, option 1, I don't need the in label in this case.

from ios-foundation.

Also in the example above it's AdminHeaderView who makes an action, not the delegate which is more like observer. So as for me it makes more sense to have adminHeaderView prefix in delegate function names.

from ios-foundation.

I like this, we should update the public API's for frameworks such as Spots to be more scope with relevant prefixes. I guess that could go into the 6.0.0 release. What do you guys think?

from ios-foundation.

The methods in question here ended up looking like this:

func spotable(_ spot: Spotable, itemSelected item: Item)
func spotablesDidChange(_ spots: [Spotable])
func spotable(_ spot: Spotable, willDisplay view: SpotView, item: Item)
func spotable(_ spot: Spotable, didEndDisplaying view: SpotView, item: Item)

from ios-foundation.