Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optional protocol requirements #19

Closed
benjohnde opened this issue Dec 21, 2017 · 4 comments
Closed

Optional protocol requirements #19

benjohnde opened this issue Dec 21, 2017 · 4 comments

Comments

@benjohnde
Copy link
Contributor

Any plans on using optional protocol requirements? Sometimes you only need one specific function to be invoked instead of all.

Of course, you can easily ignore the others. But wouldn't it be nicer to just use the necessary protocol requirements a specific use case demands?

@dfed
Copy link
Collaborator

dfed commented Dec 21, 2017

While I sympathize that you don't always need every method, optional protocol methods can lead to a world of hurt and aren't very swift-y (would require our protocols to inherit from NSObjectProtocol) . Having required protocols means that the compiler can enforce that you've properly implemented what you intended to implement, and enforces a stricter API.

I find the safety-blanket provided by required implementations strongly outweighs the value of optional implementations. That said, maybe our protocols are requiring too much? Are there specific protocols we have that you find you tend to only use part of? Maybe the better answer would be for us to split those protocols up into smaller bits.

@benjohnde
Copy link
Contributor Author

benjohnde commented Dec 22, 2017

Splitting up could be a solid idea. I do not really have a strong opinion on that. I am very open to new ideas. Here is something which came to my mind this morning: I think the most possible Swifty way would be to provide default implementations for the protocols via extensions. Let's just pick StateRestorationCapable for the example:

protocol StateRestorationCapable: ApplicationLaunched {
    func shouldSaveApplicationState(using coder: NSCoder) -> Bool
    func shouldRestoreApplicationState(using coder: NSCoder) -> Bool
    func willEncodeRestorableState(using coder: NSCoder)
    func didDecodeRestorableState(using coder: NSCoder)
    func viewControllerWithRestorationIdentifierPath(identifierComponents: [Any], coder: NSCoder) -> UIViewController?
}

A default implementation could look like the following:

extension StateRestorationCapable {
    func shouldSaveApplicationState(using coder: NSCoder) -> Bool {
        return true
    }
}

If you are in need of one specific protocol function, you can just override the extension or implement the protocol requirement.

As a quick demonstration (you are pleased to create a playground from scratch):

protocol StateRestorationCapable {
    func shouldSaveApplicationState() -> Bool
    func shouldRestoreApplicationState() -> Bool
}

extension StateRestorationCapable {
    func shouldSaveApplicationState() -> Bool {
        return true
    }
    
    func shouldRestoreApplicationState() -> Bool {
        return true
    }
}

class Test1: StateRestorationCapable {
    
}

class Test2: StateRestorationCapable {
    func shouldRestoreApplicationState() -> Bool {
        return false
    }
}

print(Test1().shouldRestoreApplicationState())
print(Test2().shouldRestoreApplicationState())

I am with you when talking about what does feel Swifty ❤️ and what does not. But as it is nowadays very simple to look up the API documentation (and of course you really should do beforehand you are consuming it) I think reducing boilerplate is essential.

I think both approaches:

  • i) splitting up
  • ii) providing default implementations

are solid.

In my eyes, I would go with ii) as splitting up may cause a protocol soup and with default implementations we also could add some debugging output, like print("HINT: shouldRestoreApplicationState uses default implementation").

@dfed
Copy link
Collaborator

dfed commented Dec 22, 2017

I've gone down the default implementation route before, and I believe it is more hurtful than helpful long-term since it removes any compile-time checking that you've implemented the methods you think you have (when migrating Swift versions, API versions, or just maintaining a large codebase with many developers this compile-time check is essential).

Providing default implementations of methods that have return values also makes me guess intent of the consumer, makes adoption more complex (no longer does the compiler tell you what you need to implement), and distributes the logic adoptees need to care about across packages. Today, consumers just need to be concerned with the protocols they implement (consumers shouldn't care about what SuperDelegate is doing behind the scenes). If we provided default implementations, now consumers need to check what SuperDelegate has implemented & how.

I'm curious what methods feel like boilerplate to you. Giving me a sense of where the API feels verbose will help me address the concern you're raising 🙂

@benjohnde
Copy link
Contributor Author

benjohnde commented Dec 23, 2017

Never thought of a very large codebase with multiple maintainers. In my small world the default implementation may work out, but I see and acknowledge your point. Then splitting up would be the way to go.

Concerning boilerplate, maybe I shot myself in the foot. I had this problem especially using my own LifeCycleAware implementation :) The moment you are realizing: it wasn't the best idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants