#21 Decorators are not API compatible with PluginManager

Next Major Release
accepted
None
1
2014-07-11
2014-07-05
johndoe18
No

The PluginManagerDecorator is supposed to be used for extending the PluginManger.
This is made fairly difficult by breaking API compatibility between the two interfaces.

Examples:
PluginManager accepts a plugin locator for init, Decprators do not.
PluginManager has different defaults for init than the Decorators.
PluginManager returns a list of plugins from loadPlugins, Decorators do not if they override the function.

I have only taken a short look at yapsy, so I cannot say that I have looked very much further after finding these, but I must say that from an OOP standpoint, this is very strange software design (that would partly not even be possible in strict languages for good reason).
I hope there will be an effort to keep an clean and consistent API in the future because the project looks quite useful to me.

Discussion

  • Thibauld Nion

    Thibauld Nion - 2014-07-11

    Hello,

    Thanks for you criticism. I'll try to answer the various pointsin separate comments but I'm afraid it won't reassure you 100%.

     
  • Thibauld Nion

    Thibauld Nion - 2014-07-11

    About the init of the PluginDecorator: from an OOP standpoint nothing forces a Decorator to have the same init/constructor as the class it decorates.

    Quite the opposite actually, the best for a Decorator is to take only its decorated object as argument. Opinion may differ on the fact that it is the only arguments that should be there, but that's my current personal taste.

    So the mistake here is more that the Decorator take extra args and tries to mimick the PluginManager's init (as the doc states). I'm not too happy with that but I'm keeping them in the 1.x series for backward compatibility.

     
  • Thibauld Nion

    Thibauld Nion - 2014-07-11

    About the different default values, if you digg enough in the code you should realize that they are actually the same, but this became a little hidden after a few internal refactoring. They are also kept for backward compatibility.

    However, you made me realize that there is an even more obvious bug about these default values, so I openned a ticket [bugs:#23]

     

    Related

    Bugs: #23

  • Thibauld Nion

    Thibauld Nion - 2014-07-11

    About "PluginManager returns a list of plugins from loadPlugins, Decorators do not if they override the function.".

    Well this is the fate of Python, right ? The only sure way to prevent this by design is to use one of those "stricter" languages as you mean.

    You'll have to be careful when designing or choosing a decorator for the PluginManager.

     
  • Thibauld Nion

    Thibauld Nion - 2014-07-11

    About the general remarks on the design of yapsy: yes it's a bit strange and amateurish and reflects more my "design skills" of the beginner I was more than 7years ago,

    But somehow and to my own surprise it proved "good enough" for the people who used it in their projects since it became opensource.

    I decided long ago to maintain the backward compatibility as much as possible for the 1.x release series while fixing bugs as a "service" to people who integrated yapsy in their project and didn't their project to break, This also means that a great deal of not-so-wise design choices are to remain.

    I'm not coming back on this decision by the way, but this doesn't prevent me from allowing improvements on yapsy's design when we can prevent a significant API break. For instance, the work done on PluginLocator strategies provides a much cleaner way to change the plugin location detection mechanisms than Decorators.

    I'd be very happy also to see a Yapsy v2 being developped (see [feature-requests:#3] for another thread on this subject), but I have currently no time to start this alone.

     

    Related

    Feature Requests: #3

  • Thibauld Nion

    Thibauld Nion - 2014-07-11
    • status: open --> accepted
    • assigned_to: Thibauld Nion
    • Group: Next Minor Release --> Next Major Release
     
  • johndoe18

    johndoe18 - 2014-07-11

    Thank you for this lengthy reply!

    Good enough is certainly a valid point after all, I am not complaining about yapsy not working. As a matter of fact I will stick with it throughout my project. The thing is, if it had been a project with a professional application, the current code base would probably have discouraged my away from it.
    The point you are trying to make by using the phrase "the fate of python" is actually a what I was trying to say in my original comment: It is true that you cannot guarantee function results and behavior as much as in laguages with static checks, but that does not mean you cannot solve that problem by taking extra care when it comes to implementation details. I would always expect that a public api of a wrapper objector behaves transparently towards the outside caller. The fact that you drop this compatibility without documentation does not look like a design decision, it looks like sloppy implementation. If you look a little closer you'll find a lot more details like VersionedPluginManager calling setPluginInfoClass in its constructor, which is a deprecated function. Taken the sum of those things (without the feedback you have given here) and you might arrive at the conclusion that you update/change things without the necessary care.

    To sum up: Code quality can affect project reputation because there are people around who do like to dig into the details of the stuff they are using ;)

    Still, thanks for putting this project together. Even though criticism is always easier to articulate than praise, I do really appreciate the effort.

     


Anonymous

Cancel  Add attachments





Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:





No, thanks