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

Declarations from extensions cannot be overridden yet #1

Closed
IsmailHassanein opened this issue Jul 18, 2017 · 6 comments
Closed

Declarations from extensions cannot be overridden yet #1

IsmailHassanein opened this issue Jul 18, 2017 · 6 comments

Comments

@IsmailHassanein
Copy link

Hi Aleksandar,
Thank you for the great coordinator. I am trying to follow your steps in the example.
You have added for example:

extension UIResponder {
	///	Adds given Product to the temporary cart and immediatelly shows Payment screen 
	func cartBuyNow(_ product: Product, sender: Any?) {
		coordinatingResponder?.cartBuyNow(product, sender: sender)
	}

	///	Adds given Product to the cart
	func cartAdd(product: Product, color: ColorBox, sender: Any?, completion: (Bool) -> Void) {
		coordinatingResponder?.cartAdd(product: product, color: color, sender: sender, completion: completion)
	}

	///	show/hide the cart
	func cartToggle(sender: Any?) {
		coordinatingResponder?.cartToggle(sender: sender)
	}
}

Then in the applicationCoordinator you override them:
```
// UIResponder coordinating messages

override func cartBuyNow(_ product: Product, sender: Any?) {
	//	re-route to CartManager and/or CartCoordinator

	let ac = UIAlertController(title: nil, message: "cart-BuyNow", preferredStyle: .alert)
	ac.addAction( UIAlertAction(title: "OK", style: .default) )
	rootViewController.present(ac, animated: true)
}

override func cartAdd(product: Product, color: ColorBox, sender: Any?, completion: (Bool) -> Void) {
	//	re-route to CartManager and/or CartCoordinator

	let ac = UIAlertController(title: nil, message: "cart-Add", preferredStyle: .alert)
	ac.addAction( UIAlertAction(title: "OK", style: .default) )
	rootViewController.present(ac, animated: true)
}

override func cartToggle(sender: Any?) {

	let ac = UIAlertController(title: nil, message: "cart-Toggle", preferredStyle: .alert)
	ac.addAction( UIAlertAction(title: "OK", style: .default) )
	rootViewController.present(ac, animated: true)
}
With no problems at all.

When I try to do the same I get the above error. Could you please point to me what I am missing? 

Thank you.
@IsmailHassanein
Copy link
Author

I found the problem. I was passing a variable of type isn't NSObject subclass. I got it when I tried to put the keyword @objc before the function, then I got an error that param 1 is not objective c representable.

Thank you.

@radianttap
Copy link
Owner

Yes, because it's all based on UIResponder. Add dynamic before coordinatingResponder methods and compiler will inform you about issues with non-ObjC types as soon as you write them.

	dynamic func cartBuyNow(_ product: Product, sender: Any?) {
		coordinatingResponder?.cartBuyNow(product, sender: sender)
	}

@xieweizhi
Copy link
Contributor

@radianttap , I think it's a Swift 4 issue. Will you consider update the project to support Swift 4?

@radianttap
Copy link
Owner

Library itself is working in both Swift 3 / 4. I have updated the demo project and used pods to Swift 4.
There's a separate swift3 branch in case you need to try it out.

@falsecrypt
Copy link

I see some problems however in your (@radianttap) implementation of the "Coordinator" pattern. For example you are using UIResponder extensions and not Delegates to connect UIViewControllers and Coordinators. Here you must use @objc dynamic func to make it work but it means we also must use objc parameters only and not e.g. Swift Enums because of errors like Method cannot be marked @objc because the type of the parameter 2 cannot be represented in Objective-C.
Secondly you are polluting the interfaces of all objects that inherit from UIResponder with domain-specific methods like fetchPromotedProducts and making them accessible from e.g. all UILabels : nameLabel.fetchPromotedProducts[...] which compiles fine but makes no sense at all and of course UILabels should not be aware of any kind of domain/business logic.
This is not a complete list with problems only what i've seen so far.

@radianttap
Copy link
Owner

radianttap commented Jan 27, 2019

I have discussed this in the very first post I wrote about the pattern implementation, see Downsides at the end of this post.

Here you must use @objc dynamic func to make it work but it means we also must use objc parameters only and not e.g. Swift Enums because of errors like Method cannot be marked @objc because the type of the parameter 2 cannot be represented in Objective-C.

This is solved through boxing. See for example ColorBox in Color.swift in the example app.

Secondly you are polluting the interfaces of all objects that inherit from UIResponder with domain-specific methods like fetchPromotedProducts and making them accessible from e.g. all UILabels : nameLabel.fetchPromotedProducts[...] which compiles fine but makes no sense at all and of course UILabels should not be aware of any kind of domain/business logic.

Yep. Hence my continued emphasys on common sense. I don't attempt to save the developer from themself. Nor I think it's actually technically feasible to limit visibility of these methods while keeping the good parts.

This is not a complete list with problems only what i've seen so far.

I personally don't see these as problems – it's a deliberate choice on my part, a trade off between usability and what I can do in UIKit while working from outside of UIKit team.
(You may feel differently of course.)

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

4 participants