Kevin Carwile Posted June 4, 2017 Posted June 4, 2017 With the decision to use traits in IPS 4.2, how are 3rd party apps supposed to extend any functionality contained within them? In an attempt to update Automation Rules for compatibility with IPS 4.2, I need to hook into an \IPS\Content\Reactable method to attach an event dispatcher so that rules can be triggered when content is reacted upon. By using traits, the functionality is now basically locked out of being extended or hooked into.
CodingJungle Posted June 4, 2017 Posted June 4, 2017 well not at the \IPS\Content, \IPS\Content\Item and \IPS\Content\Comment, you would need to do it in like \IPS\forums\Topic\Post to be able to overload it (as the trait would overload your overload via a hook). if they insist on using traits, i would suggest they make them merely wrappers for other classes, like move all the reactable methods to say reactableClass.php, then in reactable just do: public function removeReaction( \IPS\Member $member = null ){ \IPS\Content\Classes\reactableClass::removeReaction( $member ); } that way we can hook into reactableClass.php and the trait could be used as normal.
Kevin Carwile Posted June 4, 2017 Author Posted June 4, 2017 1 hour ago, CodingJungle said: if they insist on using traits, i would suggest they make them merely wrappers for other classes I agree. I can't think of any other way to remove the impedance that this introduces. Although, there is another problem that I can think of. You will not be operating internal to the class that uses the trait so therefore protected properties and methods will be unavailable to the external class.
CodingJungle Posted June 4, 2017 Posted June 4, 2017 28 minutes ago, Kevin Carwile said: I agree. I can't think of any other way to remove the impedance that this introduces. Although, there is another problem that I can think of. You will not be operating internal to the class that uses the trait so therefore protected properties and methods will be unavailable to the external class. yeah its not a perfect idea, but its the price we pay living in a single inheritance language lol
Kevin Carwile Posted June 4, 2017 Author Posted June 4, 2017 3 minutes ago, CodingJungle said: yeah its not a perfect idea, but its the price we pay living in a single inheritance language lol I don't like paying prices that don't need to be paid. The existing strategy of placing interface functionality in a base class, but allowing child classes to implement the interface to enable that functionality has allowed the "interior decorator" design pattern (the hooks system) to work well. By switching to traits, you trade the frameworks greatest strength for a relatively non-beneficial ability to decouple from the base class. That's not a very good tradeoff.
CodingJungle Posted June 4, 2017 Posted June 4, 2017 take the report system, if i wanted to use that and not use content/item/comment classes, i have to reproduce all that code for it. with a trait, i'd just call the trait... you also have like the 8k lines that Items.php has, would be greatly reduced. which would optimize and reduce memory usage for the core classes if traits were used. (i know not by much, but every little bit can help, and IPS is already heavy). so there are trade offs to the "interior decorator" design pattern as well.
Kevin Carwile Posted June 4, 2017 Author Posted June 4, 2017 23 minutes ago, CodingJungle said: so there are trade offs to the "interior decorator" design pattern as well. Right, so the question is if a slightly more organized codebase is worth trading for the openness and flexibility of the product to be extended in new ways. Because I can tell you right now. There is no way to hook into a trait to provide enhanced functionality without knowing the specific base class that is going to use it. So instead of having one hook on the base implementation, you have to hook into specific child classes that use it (which is not practical when your enhancement is abstract) such as Automation Rules, which needs to trigger an event for any arbitrary class that uses the trait. End result is either no enhancement of the base product, or way more hooks than necessary on every conceivable content item that may use it (how's that for a performance gain). BomAle 1
Midnight Modding Posted June 4, 2017 Posted June 4, 2017 (edited) I haven't worked with 4.2 yet, but I definitely see the problem here if they put a lot of functionality that third parties are likely to want to manipulate in traits. (I obviously don't know their exact setup on it, though, since I have done nothing in 4.2). Edited June 4, 2017 by Midnight Modding
CodingJungle Posted June 4, 2017 Posted June 4, 2017 1 minute ago, Kevin Carwile said: Right, so the question is if a slightly more organized codebase is worth trading for the openness and flexibility of the product to be extended in new ways. i think the right question would be, what can be done to satiate both requirements? i would like to use some of these things that traits would allow me too without using an item's class, and allow you to abstract to the metal. BomAle 1
Management Solution Lindy Posted June 20, 2017 Management Solution Posted June 20, 2017 This is being discussed with development, I just wanted to let you know we're not ignoring it. One proposed solution moving forward is to incorporate our own event trigger system - for example, when a reaction is provided, send a "reactionGiven" event that you can then work with. This allows you extendability without us needing to get hacky. Once the core devs have given more insight, I'll update this so we're all on the same page. CodingJungle, Ilya Hoilik, TSP and 4 others 7
CodingJungle Posted June 20, 2017 Posted June 20, 2017 3 minutes ago, Lindy said: incorporate our own event trigger system this could be useful else where too, just saying PrettyPixels 1
Management Lindy Posted June 20, 2017 Management Posted June 20, 2017 28 minutes ago, CodingJungle said: this could be useful else where too, just saying Agreed. BomAle and CodingJungle 2
HeadStand Posted June 20, 2017 Posted June 20, 2017 On 6/4/2017 at 7:08 PM, CodingJungle said: i think the right question would be, what can be done to satiate both requirements? i would like to use some of these things that traits would allow me too without using an item's class, and allow you to abstract to the metal. What if the event trigger method is on the item (or comment)? So IPS\Content\Item would have the initial onReactionGiven method. That would allow you to hook into the base level AND have access to protected properties. Or am I missing something completely?
Kevin Carwile Posted June 21, 2017 Author Posted June 21, 2017 Its a fundamental problem and an engineering trade off. Using traits to encapsulate functionality decouples that functionality from any particular class (which improves reusability), but at the same time locks it from being extended using the hooking system that has made the IPS4 platform so flexible (which decreases 3rd party extensibility). I think traits are a bigger negative to the platform than a positive. Having to switch from a decorator design pattern to an observer design pattern is a step backwards. InvisionHQ 1
Joel R Posted June 21, 2017 Posted June 21, 2017 How I imagine every third-party dev now after reading Kevin's post My guest room could use some help. That is all. Back to your regularly scheduled programming of #devtalk
HeadStand Posted June 21, 2017 Posted June 21, 2017 2 hours ago, Kevin Carwile said: I think traits are a bigger negative to the platform than a positive. Having to switch from a decorator design pattern to an observer design pattern is a step backwards. I don't know if I agree with that. I would like to see certain things de-coupled, like search and tags, for example. I'm not crazy about having to implement content items AND nodes just to get those things to work.
Kevin Carwile Posted June 21, 2017 Author Posted June 21, 2017 53 minutes ago, HeadStand said: I don't know if I agree with that. I would like to see certain things de-coupled. Until you need to decorate some of that decoupled abstract functionality. Then you'll see the problem. One approach lends to inconveniences, true. But the other way excludes possibilities.
bfarber Posted June 21, 2017 Posted June 21, 2017 We are exploring possibilities as Lindy said, including implementing more event-based triggers (which we do in some places already, such as MemberSync extensions) and looking into adjusting the hooking system to allow it to work with traits as well. As Lindy said, we will update everyone with more information in due course. InvisionHQ, DSystem, Ramsesx and 2 others 5
Numbered Posted October 27, 2017 Posted October 27, 2017 (edited) @bfarber, @Lindy - any updates? Not only for event trigger. Need some ability to hook Reactable trait methods (with and without parent calling). In some situations canReact method must working in other way (provided by plugin). And in some situations react method must worked in very other way. How i can do it now? Extend, as example, \IPS\forums\Topic\Post ? It's not clear solution and very unusable extend each react-supported types of content upd.: and some additional improvement request: Reactable trait method react throw \DomainExceptions only with messages like 'cannot_react', id string from stack 'react_daily_exceeded' - can you throw this exception with a some code as a second parameter (different codes, of course) - it will be more right and provide to us better solution for catch this exceptions in our hooks. Like example: try{ call_user_func('parent::react... } catch \DomainException ($e) { if ($e->code === 'something') { do_my_stuff... } Thanks! Edited October 27, 2017 by Upgradeovec
Adriano Faria Posted February 21, 2018 Posted February 21, 2018 (edited) On 21/06/2017 at 9:57 AM, bfarber said: We are exploring possibilities as Lindy said, including implementing more event-based triggers (which we do in some places already, such as MemberSync extensions) and looking into adjusting the hooking system to allow it to work with traits as well. As Lindy said, we will update everyone with more information in due course. Any change in this for 4.3? I need to hook into canReact(). Edited February 21, 2018 by Adriano Faria
MIXOH Posted April 16, 2018 Posted April 16, 2018 any updates or info from respected dev? Use trait it's good from dev, but not for third-party. Any methods for hook trait.
DSystem Posted April 16, 2018 Posted April 16, 2018 I am also waiting for a solution.? https://ipsguru.net/forums/topic/661-automation-rules-full-140-and-reactions-ips-42-beta
A Zayed Posted July 12, 2018 Posted July 12, 2018 Is there any plan to allow the extendability for traits?
teraßyte Posted October 30, 2018 Posted October 30, 2018 (edited) I guess it's about time to send a rescue team? The explorers got lost along the way it seems... 🙄 Jokes aside. I found myself needing to hook into one of the reactions functions and remembered this topic. It seems nothing has changed since then. Edited October 30, 2018 by teraßyte Adriano Faria, Cyboman and DSystem 3
HeadStand Posted October 31, 2018 Posted October 31, 2018 On 10/30/2018 at 2:56 PM, teraßyte said: Jokes aside. I found myself needing to hook into one of the reactions functions and remembered this topic. It seems nothing has changed since then. You CAN hook into it, as long as it's on a specific class. Meaning, you can hook into \IPS\forums\Topic, just not \IPS\Content\Reactable. Is that not sufficient for what you're doing? Ryan Ashbrook 1
Recommended Posts