On 29/04/12 19:16 +0200, Albert Cervera i Areny wrote: > A Dimecres, 25 d'abril de 2012 13:46:18, Cédric Krier va escriure: > > On 25/04/12 11:37 +0000, [email protected] wrote: > > > Please review this at http://codereview.tryton.org/346003/ > > > > This is a first draft for our next step to the Active Record pattern. > > Let's talk here about the design. > > Here are the main changes: > > > > * Add RPC > > * Repace BrowseRecord by Model instance > > * Replace Cache decorator by a simple LRU Cache > > * Remove Cacheable > > * Remove _description > > * Rename _name by __name__ > > * Use class in Pool > > I like the new design: Removing _description for __doc__ looks nice. The same > for using the class in Pool or replacing _name by __name__. Just several > comments: > > It has its technical reasons, but it is a bit complicated to figure out how > each kind of function should be defined. For example, default_* use > @staticmethod, get/set and on_change use standard functions (but may > use > @classmethod), while check_*, read, write, etc must use @classmethod. When > you > think of it, you can find out why each one uses each kind of definition, but > It > makes coding a little bit harder, specially for newcommers... Not that I have > a better solution, but wanted to point this out.
We will update the documentation to clarify each cases. (check_* should also work like getter of fields.Function) > Also the fact that some get functions of fields.Function use @classmethod and > others use "standard" functions makes it a bit more complex. Could we > standarize and always use the same mechanism? Indeed the simplier is the instance method but that is not always the best because it could be better to work on list like when you can do a SQL query to compute all at once. > Is there a change to keep the Cache decorator? I think it makes code cleaner. I don't think. There was an issue with the decorator see issue1825 [1]. Also the code of the decorator was a little bit magic. And I'm pretty sure we will find new way to use this implementation (already it merge the decorator with the Mixin). > I know we prefer explicit vs implicit but it be nice if we didn't have to > register all classes :) But it is more powerful because if we want to follow the module organisation (extension in file named like module), we must be able to give a register order different than the importation one. Also it will be possible to register generated classes or based on condition. Other minor advantage, it remove pyflakes warnings :-) [1] https://bugs.tryton.org/issue1825 -- Cédric Krier B2CK SPRL Rue de Rotterdam, 4 4000 Liège Belgium Tel: +32 472 54 46 59 Email/Jabber: [email protected] Website: http://www.b2ck.com/
pgp5D7SnqDvZc.pgp
Description: PGP signature
