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

[Request | Suggestion] Protocol extension hides compiler errors #25

Open
esttorhe opened this issue May 2, 2016 · 4 comments
Open

[Request | Suggestion] Protocol extension hides compiler errors #25

esttorhe opened this issue May 2, 2016 · 4 comments

Comments

@esttorhe
Copy link
Member

esttorhe commented May 2, 2016

I was trying to implement the router on an app and it kept crashing with the error that I wasn't implementing the Routable methods which I had or I thought I had.

The signature was wrong on my implementation and thanks to this extension https://github.com/ReSwift/ReSwift-Router/blob/master/ReSwiftRouter/Routable.swift#L31-L55

The compiler couldn't tell me that was the case because the extension was already providing conformance to the protocol.

Wouldn't it make more sense to remove said extension and let the compiler complain about the lack of conformance to that protocol so newcomers can adopt it more easily?

Maybe there's a bigger issue behind the reasoning of adding this default conformance and I just don't know about it but from my experience having the compiler complain about the wrong signature would have made this easier to track 😄

@Ben-G
Copy link
Member

Ben-G commented May 2, 2016

@esttorhe I'm sorry you ran into this. I planned on removing the default implementations for a while but didn't get around to it.

The idea is that some of the methods are "optional". You might have a route that can push a segment, but not replace an existing segment. So not all routables need to implement all methods.

The correct way would be to provide 3 different protocols and I think that's what we should do ASAP. The current solution was a quick hack and should be removed.

@esttorhe
Copy link
Member Author

esttorhe commented May 2, 2016

No need to apologize Ben; I understand this framework is still taking shape.

I just wanted to point this out in case it was a bug/unimplemented piece of code (which in a way it seems it is).

I'll leave this ticket open to track progress of this them.

I thought some could be optional though 😄

🙇

@smebberson
Copy link

smebberson commented Oct 7, 2016

I actually just hit this too, and this has given me a clue about where to go to try and fix it.

@frranck
Copy link
Contributor

frranck commented Oct 27, 2016

Got the same issue, because my function was declared as:

 func pushRouteSegment(
        routeElementIdentifier: RouteElementIdentifier,
        animated: Bool,
        completionHandler: @escaping RoutingCompletionHandler) -> Routable

instead if

    func pushRouteSegment(
        _ routeElementIdentifier: RouteElementIdentifier,
        animated: Bool,
        completionHandler: @escaping RoutingCompletionHandler) -> Routable

Would be great to update the examples...

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

No branches or pull requests

4 participants