Open/Closed principle?
Moderator: General Moderators
Open/Closed principle?
In Magento there are a bunch of classes
Extension developers override these classes and override methods to change behavior
Magento also provides an event architecture, which allows multiple extension developers to utilize the same "hook point".
It would be impossible to develop a system and identify every hook that is needed from the get-go.
If 2 developers try to override a class, instead of using a hook point, only 1 would take effect.
Is this an example of open/closed principle? (ex. a developer would not feel the need to override said method if it followed open/closed?). I was thinking about writing a "Magento killer" some day, and I think that solving this problem would definitely be a good start.
So my question is how would you guys design said system? Would you just try to be really attentive to developer's requests to add hooks? (Magento has a thread where we can make requests but they ignore us now-adays). Anyways an architectural solution would be desirable. The Magento forum people have suggested firing events for EVERY method reflectively, I think that is bad suggestion for performance reasons.
Is aspect oriented programming the only solution? Aside from just adding hooks as I go, as I discover people needing them (knowing that a certain % won't realize I am willing to add the hooks, and would possibly just "give up" instead of making the request?)
Extension developers override these classes and override methods to change behavior
Magento also provides an event architecture, which allows multiple extension developers to utilize the same "hook point".
It would be impossible to develop a system and identify every hook that is needed from the get-go.
If 2 developers try to override a class, instead of using a hook point, only 1 would take effect.
Is this an example of open/closed principle? (ex. a developer would not feel the need to override said method if it followed open/closed?). I was thinking about writing a "Magento killer" some day, and I think that solving this problem would definitely be a good start.
So my question is how would you guys design said system? Would you just try to be really attentive to developer's requests to add hooks? (Magento has a thread where we can make requests but they ignore us now-adays). Anyways an architectural solution would be desirable. The Magento forum people have suggested firing events for EVERY method reflectively, I think that is bad suggestion for performance reasons.
Is aspect oriented programming the only solution? Aside from just adding hooks as I go, as I discover people needing them (knowing that a certain % won't realize I am willing to add the hooks, and would possibly just "give up" instead of making the request?)
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Open/Closed principle?
This sounds like there are two overlapping systems -- each trying to follow the open/closed principle -- but trampling over each other. If you extend a class that can then be accessed by a hook point then you have "opened" the source to a change from the hook point's perspective.josh wrote:In Magento there are a bunch of classes
Extension developers override these classes and override methods to change behavior
Magento also provides an event architecture, which allows multiple extension developers to utilize the same "hook point".
It would be impossible to develop a system and identify every hook that is needed from the get-go.
If 2 developers try to override a class, instead of using a hook point, only 1 would take effect.
Is this an example of open/closed principle? (ex. a developer would not feel the need to override said method if it followed open/closed?). I was thinking about writing a "Magento killer" some day, and I think that solving this problem would definitely be a good start.
I don't know enough about Magento to know the problem. It is either a messy problem that unfortunately has a messy solution (i.e. complex and detailed), or you need to take one more step back from the problem and understand what is really needed -- and build in that more basic design solution.josh wrote:So my question is how would you guys design said system? Would you just try to be really attentive to developer's requests to add hooks? (Magento has a thread where we can make requests but they ignore us now-adays). Anyways an architectural solution would be desirable. The Magento forum people have suggested firing events for EVERY method reflectively, I think that is bad suggestion for performance reasons.
AOP doesn't sound right because these are not cross-cutting concerns. The "event for every method" idea sounds like implementing an AOP like system, but it sounds wrong to me. I also wonder if everything really needs the same solution. Did they keep adding hooks because a hook solved the last problem, not because it was right for this problem? I think there may be several "architectural solution" necessary. You should read about SOLID which puts open/closed in context.josh wrote:Is aspect oriented programming the only solution? Aside from just adding hooks as I go, as I discover people needing them (knowing that a certain % won't realize I am willing to add the hooks, and would possibly just "give up" instead of making the request?)
(#10850)
Re: Open/Closed principle?
For a hook based system, I think the solution would be to allow developers to add custom hooks themselves for the plug-ins they make (similar to how jQuery allows you to define custom events).
Re: Open/Closed principle?
Well let me elaborate on what the problem is and maybe you can provide explanations of how a solution would work (and how you arrived there).
Basically lets say you've got a Cart class. Inside your controller you instantiate and call some methods to update quantities and such. Now lets say some 3rd party wants to develop an extension for your code. One that would keep a log of every quantity change that took place. Your cart object has a seam for that already, but they shouldn't have to edit that code to change it (open/closed and no way to distribute extension).
In Magento they instantiate all objects thru a factory. Your module can "ask" to have the system instantiate a different class name.
Example
Mage::model('catalog/product')
would find a class
Mage_Catalog_Product
Unless you're running my extension in which case
My_Extension_Catalog_Product would get instantiated. That class would extend the "core" one.
The problem is now you can't have multiple modules that affect functionality in the same part of the system (no multiple inheritance).
The second solution is they invert the control and fire events in that "quantity change" method. Now modules can "listen" on the event (think callback), this allows any number of modules to be invoked anytime an "event" in the system occurs.
So the event solution pretty much seems like it solves the whole problem. My question then would be how do you give them enough hooks, without giving too many? Couldn't I just look at any method in a given system and contrive (a very real) scenario where I'd want to change it and package that up as an extension and send it to some one?
Maybe this is why Windows is so buggy, multiple software trying to always modify the look & feel & experience. Supposedly the creator of PHPUnit is working on the same issue in his platform http://www.oxid-esales.com/
http://joshribakoff.com/?p=81
If you read my blog post in detail I offer another solution involving cloning the object and it's state, and temporarily transferring execution to the cloned object, and then reflecting back it's state when the method is done. It relied on __call and I provided example code for Magento (bug its very messy anyways)
Basically lets say you've got a Cart class. Inside your controller you instantiate and call some methods to update quantities and such. Now lets say some 3rd party wants to develop an extension for your code. One that would keep a log of every quantity change that took place. Your cart object has a seam for that already, but they shouldn't have to edit that code to change it (open/closed and no way to distribute extension).
In Magento they instantiate all objects thru a factory. Your module can "ask" to have the system instantiate a different class name.
Example
Mage::model('catalog/product')
would find a class
Mage_Catalog_Product
Unless you're running my extension in which case
My_Extension_Catalog_Product would get instantiated. That class would extend the "core" one.
The problem is now you can't have multiple modules that affect functionality in the same part of the system (no multiple inheritance).
The second solution is they invert the control and fire events in that "quantity change" method. Now modules can "listen" on the event (think callback), this allows any number of modules to be invoked anytime an "event" in the system occurs.
So the event solution pretty much seems like it solves the whole problem. My question then would be how do you give them enough hooks, without giving too many? Couldn't I just look at any method in a given system and contrive (a very real) scenario where I'd want to change it and package that up as an extension and send it to some one?
Maybe this is why Windows is so buggy, multiple software trying to always modify the look & feel & experience. Supposedly the creator of PHPUnit is working on the same issue in his platform http://www.oxid-esales.com/
http://joshribakoff.com/?p=81
http://www.phpbuilder.com/columns/Andre ... 10710.php3 (doesnt really answer any questions tho)A solution might be to copy Oxid Ecommerces override model.
In a nutshell, each class definition extends a none-existing pseudo class. E.g., two classes extending Mage_Core_Model_A
class Foo_Bar_Model_A extends Mage_Core_Model_A_Foo_Bar
class Much_Kiz_Model_A extends Mage_Core_Model_A_Much_Kiz
Now, when the class instances are created, magento would create the missing pseudo classes on the fly
class Mage_Core_Model_A_Much_Kiz extends Foo_Bar_Model_A
class Mage_Core_Model_A_Foo_Bar extends Mage_Core_Model_A
Of course, each module class would have to stick to the API and call the parent’s method when it’s done.
In practice this would not solve all issues, but it would help to eliminate a load existing conflicts, and it would create a path for modules to be created in a more compatible way in future.
Vinai
Comment by Vinai — October 16, 2009 @ 12:18 pm
If you read my blog post in detail I offer another solution involving cloning the object and it's state, and temporarily transferring execution to the cloned object, and then reflecting back it's state when the method is done. It relied on __call and I provided example code for Magento (bug its very messy anyways)
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Open/Closed principle?
There is so much here obscuring the issue that I am not exactly sure where to start.
Also, what is a "seam" ?
I am interested in backing up and rethinking about the ecommerce problem. It there a more elegant base design that solves most of these customization issues? And is there a good design route for solving the rest -- even if it take more code than you might want? In the past I have solve this problem with a system where essentially there were only plug-ins.
I want to note that your first example IS a classic cross cutting concert -- logging. There are no clean solutions for these things except doing things the long and correct way.josh wrote:Well let me elaborate on what the problem is and maybe you can provide explanations of how a solution would work (and how you arrived there).
Basically lets say you've got a Cart class. Inside your controller you instantiate and call some methods to update quantities and such. Now lets say some 3rd party wants to develop an extension for your code. One that would keep a log of every quantity change that took place. Your cart object has a seam for that already, but they shouldn't have to edit that code to change it (open/closed and no way to distribute extension).
Also, what is a "seam" ?
This seems like a pretty clear example of convenience limiting flexibility down the road. As soon as I see Mage::model('catalog/product') I know that there will be trouble later. If they simply extended your class then it seems like it would work fine.josh wrote:In Magento they instantiate all objects thru a factory. Your module can "ask" to have the system instantiate a different class name.
Example
Mage::model('catalog/product')
would find a class
Mage_Catalog_Product
Unless you're running my extension in which case
My_Extension_Catalog_Product would get instantiated. That class would extend the "core" one.
The problem is now you can't have multiple modules that affect functionality in the same part of the system (no multiple inheritance).
This sounds like using a sledgehammer to solve every problem. What events are really needed?josh wrote:The second solution is they invert the control and fire events in that "quantity change" method. Now modules can "listen" on the event (think callback), this allows any number of modules to be invoked anytime an "event" in the system occurs.
So the event solution pretty much seems like it solves the whole problem. My question then would be how do you give them enough hooks, without giving too many? Couldn't I just look at any method in a given system and contrive (a very real) scenario where I'd want to change it and package that up as an extension and send it to some one?
Again, that sounds like over-engineering a solution that over-engineering (in Magento) caused in the first place.josh wrote:If you read my blog post in detail I offer another solution involving cloning the object and it's state, and temporarily transferring execution to the cloned object, and then reflecting back it's state when the method is done. It relied on __call and I provided example code for Magento (bug its very messy anyways)
I am interested in backing up and rethinking about the ecommerce problem. It there a more elegant base design that solves most of these customization issues? And is there a good design route for solving the rest -- even if it take more code than you might want? In the past I have solve this problem with a system where essentially there were only plug-ins.
(#10850)
Re: Open/Closed principle?
What do you mean by "long and correct" way?
Almost all extensions are cross cutting concerns then. So ideally we would all just use aspect oriented programming?
Every week someone posts in the PHP code section because one of those callbacks doesn't quite do what they need (so in other words the 3rd party extension developers and the Drupal developers weren't effectively able to bridge the communication gap in order to find out which hooks are really needed).
For example volomike needed to change an existing "widget" so it only displayed content for a certain category. Since there was no hook for that the solution involved regexing the out the HTML (not a clean solution from an architecture point of view). In Magento I would have basically been able to get the entire system to start instantiating my widget instead, and I could make my widget extend the base widget, overriding one of the protected methods for example.
So when you say I am "solving the wrong problem", are you implying that instead of doing it the "Magento" way, or hard coding class names, you basically write all of your OWN code as a plugin, then 3rd party developers can write installers that disable your core plugin, and enable their enhanced one?
Here's another example:
All my CRUD screens are really LEDC (list edit delete create). On my list screens I have columns. An extension developer wants to add a column, how does he do so?
A) The architect could put a hook there ahead of time, so any # of modules can "tap into" that code.
B) Or the extension developer could be forced to entirely replace that grid object with a new grid object.
I guess my 2 questions are, is there any option "C" besides Aspect Oriented Programming (like a good way to emulate it in PHP?)
In option "A" is there any easy way to avoid totally missing the bullz-eye so to speak, in guessing which hooks would be needed?
"seam", coined by Michael Feathers in "Working effectively with legacy code" (as far as I know) is a place you can change behavior without changing the code. A parametrized method is a seam because by changing the parameter different code may run. Code in the middle of a huge "transaction script" is no seam because there's no way to call it, or change it.arborint wrote:Also, what is a "seam" ?
Almost all extensions are cross cutting concerns then. So ideally we would all just use aspect oriented programming?
How so? Then all the places they hard coded the base class would have to be renamed? That opens the doors to A) extensions can't change the same parts of the system, and B) your extension is no longer plug & play, and C) you're going to have a burden of mucking with it every time they release a new version (go back thru and rename the base class' wherever it was instantiated in the core code, for all your 100s of customers.. on every upgrade?)This seems like a pretty clear example of convenience limiting flexibility down the road. As soon as I see Mage::model('catalog/product') I know that there will be trouble later. If they simply extended your class then it seems like it would work fine.
That is my question pretty much. If you follow the "belief" of incremental design you can't just guess what is needed ahead of time like that.josh wrote:This sounds like using a sledgehammer to solve every problem. What events are really needed?
I don't think this is specific to e-commerce even, look at CMS like drupal, they have tons of functional callbacks, so does Joomla, so does wordpress, etc..josh wrote:I am interested in backing up and rethinking about the ecommerce problem. It there a more elegant base design that solves most of these customization issues? And is there a good design route for solving the rest -- even if it take more code than you might want? In the past I have solve this problem with a system where essentially there were only plug-ins.
Every week someone posts in the PHP code section because one of those callbacks doesn't quite do what they need (so in other words the 3rd party extension developers and the Drupal developers weren't effectively able to bridge the communication gap in order to find out which hooks are really needed).
For example volomike needed to change an existing "widget" so it only displayed content for a certain category. Since there was no hook for that the solution involved regexing the out the HTML (not a clean solution from an architecture point of view). In Magento I would have basically been able to get the entire system to start instantiating my widget instead, and I could make my widget extend the base widget, overriding one of the protected methods for example.
So when you say I am "solving the wrong problem", are you implying that instead of doing it the "Magento" way, or hard coding class names, you basically write all of your OWN code as a plugin, then 3rd party developers can write installers that disable your core plugin, and enable their enhanced one?
Here's another example:
All my CRUD screens are really LEDC (list edit delete create). On my list screens I have columns. An extension developer wants to add a column, how does he do so?
A) The architect could put a hook there ahead of time, so any # of modules can "tap into" that code.
B) Or the extension developer could be forced to entirely replace that grid object with a new grid object.
I guess my 2 questions are, is there any option "C" besides Aspect Oriented Programming (like a good way to emulate it in PHP?)
In option "A" is there any easy way to avoid totally missing the bullz-eye so to speak, in guessing which hooks would be needed?
Re: Open/Closed principle?
Maybe this post will be easier to see the problem distilled.
Open/Closed principle states a class should be closed to change, but open to extension
If a 3rd party developer extends a class, how does he get his new class into all the "corners" of your app? (where it was hard coded)
Open/Closed principle states a class should be closed to change, but open to extension
If a 3rd party developer extends a class, how does he get his new class into all the "corners" of your app? (where it was hard coded)
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Open/Closed principle?
There are too many questions in your first post to answer, and I am not sure many of them actually relate to the problem.josh wrote:Maybe this post will be easier to see the problem distilled.
Open/Closed principle states a class should be closed to change, but open to extension
If a 3rd party developer extends a class, how does he get his new class into all the "corners" of your app? (where it was hard coded)
I think the main problem here is that you don't want to follow the Open/Closed principle. By your statement above, you want these base classes that are in "all the corners" of the app to be open to change. And perhaps implicitly by you desire to allow multiple layers of developers to make changes -- you don't want them extensible at all.
AOP may lead to more problems than it solves. I mainly say that because I have the feeling you are headed the wrong direction. If things like logging are all you need then maybe it makes sense.
When I said "long and correct" I meant that sometimes the best solution is not the simplest and most elegant idea. Sometimes it takes more work to implement it, but the benefits are enough to make the extra work worth it. That's why I was thinking that perhaps building the system where everything was a "plugin" so that the only way to do anything was in a way that, by definition, is changeable. You provide the starting set of plugins for the default behavior and others take it from there, but all by registration.
(#10850)
Re: Open/Closed principle?
I do want to follow it, I thinkarborint wrote:I think the main problem here is that you don't want to follow the Open/Closed principle.
No, only extension (I assume change is defined as going in and changing the code). In Magento I can extend a class without changing any core code.By your statement above, you want these base classes that are in "all the corners" of the app to be open to change.
"plugin" is a very general concept. Could you explain tangible terms how an extension developer would add functionality onto an existing plugin in such a hypothetical system? Would it work like the example in my last post? They would disable the "core plugin", and create a new "extended plugin" with a different name, which would extend the "core plugin"?When I said "long and correct" I meant that sometimes the best solution is not the simplest and most elegant idea. Sometimes it takes more work to implement it, but the benefits are enough to make the extra work worth it. That's why I was thinking that perhaps building the system where everything was a "plugin" so that the only way to do anything was in a way that, by definition, is changeable. You provide the starting set of plugins for the default behavior and others take it from there, but all by registration.
Do you have any examples of code that implements the concept well? Preferably in PHP?
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Open/Closed principle?
I don't have much time right now to think about this, but off the top of my head my idea is that everything in the system I am thinking about is something like an Intercepting Filter chain. By default the chains have the base behavior installed. You can add as many other "filters" before and after the standard behavior -- or modify replace it if necessary. It is a very rough sketch admittedly, but hopefully it gives the idea.josh wrote:"plugin" is a very general concept. Could you explain tangible terms how an extension developer would add functionality onto an existing plugin in such a hypothetical system? Would it work like the example in my last post? They would disable the "core plugin", and create a new "extended plugin" with a different name, which would extend the "core plugin"?
Do you have any examples of code that implements the concept well? Preferably in PHP?
(#10850)
Re: Open/Closed principle?
Ok well I guess I get that part.
Sorry to give a moving target but I guess my question is this then. What heuristic do you use to decide which parts of the system are "pluggable"
The reason I ask, is it sounds like only certain parts of the system would be "pluggable", and couldn't I then just come up with any idea for a plugin in that system that is not possible to implement?
Unless someone can extend *any* class in any corner of the app, we are imposing limitations on what they can develop, am I right in assuming that?
This is a stupid example but lets say you wrote a system that supported plugins, how would I for example... write a plugin that modifies how the plugin architecture works? Even though that sounds crazy I can do that in Magento, and I've found a need for it, when the plugin system imposed limitations on me, it still at least provided enough functionality for me to change the whole plugin architecture of Magento, without hacking core code.
Another example, what if someone wants to replace a data access object? or replace a view helper? or replace a controller? or replace an entire model? Would this be considered a change, or an extension?
Sorry to give a moving target but I guess my question is this then. What heuristic do you use to decide which parts of the system are "pluggable"
The reason I ask, is it sounds like only certain parts of the system would be "pluggable", and couldn't I then just come up with any idea for a plugin in that system that is not possible to implement?
Unless someone can extend *any* class in any corner of the app, we are imposing limitations on what they can develop, am I right in assuming that?
This is a stupid example but lets say you wrote a system that supported plugins, how would I for example... write a plugin that modifies how the plugin architecture works? Even though that sounds crazy I can do that in Magento, and I've found a need for it, when the plugin system imposed limitations on me, it still at least provided enough functionality for me to change the whole plugin architecture of Magento, without hacking core code.
Another example, what if someone wants to replace a data access object? or replace a view helper? or replace a controller? or replace an entire model? Would this be considered a change, or an extension?
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Open/Closed principle?
This simple answer is that I don't know. It sounds like Magento does most of what you need, and you give plenty examples of that. I can't tell if you are just familiar with the code and know how to do workarounds ... or if the flexibility is actually built in. I also don't know the distinction between "it lets me do" and "hacking the core" so I can't really answer any of these questions.
It seems like if you allow replacement, extension and plugins that you will inevitability have problems with things not working.
It seems like if you allow replacement, extension and plugins that you will inevitability have problems with things not working.
(#10850)
Re: Open/Closed principle?
Magento is not necessarily the answer. So I guess what you are saying is each "package" in my application should have minimal dependencies, so it can be entirely swapped out by extension developers who feel there are not enough hooks?
In Magento I would replace a specific class (or any number of specific classes). I guess in Joomla one would replace an entire module (all of it or nothing).
Is there any such thing as an in-between or a hybrid? What kinds of complications do you see it creating? Coupling between "core" code and extension code?
definitions
core = code that is part of the "original" system
hack = modifying the core code directly in such a way that the next version of the "core" could not be unzipped untop of your install, without it replacing all the local modifications you had made as an end user of this system.
In Magento I would replace a specific class (or any number of specific classes). I guess in Joomla one would replace an entire module (all of it or nothing).
Is there any such thing as an in-between or a hybrid? What kinds of complications do you see it creating? Coupling between "core" code and extension code?
definitions
core = code that is part of the "original" system
hack = modifying the core code directly in such a way that the next version of the "core" could not be unzipped untop of your install, without it replacing all the local modifications you had made as an end user of this system.
- Christopher
- Site Administrator
- Posts: 13596
- Joined: Wed Aug 25, 2004 7:54 pm
- Location: New York, NY, US
Re: Open/Closed principle?
My two main observations are:
1. I would go either the extension or plugin route, not both. My inkling is that the plugin route sounds preferable because you want improvements that are distributable.
2. It seems like your main problem is that the Magento group will not keep adding hooks. If you create your own you can be more responsive (or not
.
1. I would go either the extension or plugin route, not both. My inkling is that the plugin route sounds preferable because you want improvements that are distributable.
2. It seems like your main problem is that the Magento group will not keep adding hooks. If you create your own you can be more responsive (or not
(#10850)