Pk, 2011-10-14 20:45 +0100, Chris Morley rakstīja: > On 13/10/2011 22:04, My Th wrote: > > > > Other solution would be to change the constructor so that pointers in > > plugin map are not overwritten if the instance with the same ID already > > exists.
First task would be to solve the issue with MakeNewInstance() for recent OB versions. I proposed a solution in previous patch and I think it is a decent solution which could be implemented in next minor version of OB unless anybody knows that there is something wrong with it. Is it good enough solution for now? I have seen the proposal of new plugin architecture in wiki: http://openbabel.org/wiki/Plugins And it is one of the v3.0 goals: http://openbabel.org/wiki/Version_3.0_Projects Good ideas and some of those goals already have been reached, but it is off-topic in this discussion. I'll still make some comments :) To be honest, I don't completely grasp what the new plugin scheme implies, but after reading your mail I can see what it tries to solve. But I think it should be reconsidered if introduction of plugin factory doesn't overcomplicate things. Maybe I'm just new to the concept :) > I had been thinking previously about ways of updating the plugin system. > It could do with improvements in three areas: > a) the need to load all the dynamic libraries at startup I think my plugin loading patch does exactly that. The v3.0 project lists the goal to "Load only as needed". We need to load all the plugin libraries if we need to use at least one plugin, so I don't see how to improve it in the terms of loading only required libraries (in general we don't know in which file the plugin we need is). > b) a proper factory class for instances, rather than a macro Actually the macro in plugin.h provides constructor and some functions common to all plugins, but the global instances are made by instantiating them in respective plugin files (in practice those instances are constructed when the plugins are loaded and plugin map(s) holds pointers to them). Though there are macros for those in the case of static build and some of the plugins are not there (forgotten?): ABINITFormat CacaoInternalFormat CanSmilesNS CASTEPFormat EEMCharges FPCount GROFormat InChICompareFormat InChIKeyFormat L5 MyFpType NoCharges OpAddFileName OpGenAlias OpLargest OpNewV OpSmallest SecondOpAlign Tautomers XSFFormat XXXFormat > c) not using a single global instance when deployed > > Your recent discussion has been related to c). Most plugins use only the > single instance, which is a bit unsafe, even without multiple threads. I > wonder whether a cleaner, more radical design would be better. A global > instance of a plugin is made when the it is loaded, as at present, but > FindType(id) would always returns a shared_ptr to a new instance. The > shared_ptr means this instance would be copyable and automatically > deleted without the user having to remember. The always-new instances > would be a move towards thread safety. The user would have more > flexibility but is insulated from inadvertent damage because the global > instance is not accessible to him. Similar effect could be easily achievable in the current system by giving for each plugin sub-type a function MakeNewInstance() and making FindType() functions to return a new instance. The cleanup is a problem. It would be required for user to do it. > The code of each plugin type would not change, but it would require a > small change to the code that used the the plugin, potentially breaking > user code. > e.g. old code > Forcefield* pFF = FindForceField("UFF"); > if(pFF) { > ... > } > The user would need to replace "Forcefield*" by "shared_ptr<Forcefield>" > or "auto". I'm not sure how acceptable this is, and I haven't tested > these ideas. This looks like a good solution. But it introduces a dependency to Boost (I wonder what C++2011 has to offer here) and would not be backwards compatible, fine for v3.0 tough. Question, how acceptable your proposal for plugin architecture update is, probably could be answered by people who have greater experience with C++ and OB code. For me it looks good in context of v3.0, but in what time frame does it put then? I'm personally interested that when (now) coding (in Python or C++) against libopenbabel I shouldn't need to: - read the code of the library itself and - fix bugs in it to be able to do what it is supposed do. Both of these are great sources of frustration, but could be somewhat alleviated by more frequent releases of OB, so that there is at least a promise that someday they might go away. Keep up for the coming one! :) Reinis ------------------------------------------------------------------------------ All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity and more. Splunk takes this data and makes sense of it. Business sense. IT sense. Common sense. http://p.sf.net/sfu/splunk-d2d-oct _______________________________________________ OpenBabel-Devel mailing list OpenBabel-Devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openbabel-devel
