From: Doug B. <dou...@gm...> - 2009-12-22 14:15:41
|
On Tue, Dec 22, 2009 at 8:50 AM, Gerald Britton <ger...@gm...> wrote: > I am leaning towards grouping the "higher level" access methods in a > separate module that can be partially or wholly included as needed. if > done right we can also use them as mixins where useful. > > One thing I try to keep an eye on is the high cost of a function call > in Python. Compared to C, a Python function call is expensive. > Sometimes the lessons we learned using C or C++ have to be unlearned > to produce efficient Pythonic code. So while factoring sounds good > from a structural view it can hurt performance. > > Python is a dynamic language after all. More and more I try to > exploit that -- building functions at run-time for example. (I know I > am rambling a bit!) I'm just thinking a bit more functionally these > days. I've even toyed with the idea of porting gramps to Haskell or > Erlang just for the fun of it! This is all true. However, in the big picture these speed differences are so small as to be negligible --- when compared to what is magnitudes-more-expensive: O(n) (or worse) places, disk access, net access, graphics, etc. You could spend a lifetime re-writing Gramps in assembly language, and it would be as fast as you can make it. But someone could come along and write a better Db layer, and the Python version would be on top again. So, yes, port for the fun of it :) But I don't think we should make these kinds of decisions based on perceived speed differences of function calls. $.02 -Doug > > On 12/22/09, Doug Blank <dou...@gm...> wrote: >> On Tue, Dec 22, 2009 at 7:20 AM, Gerald Britton >> <ger...@gm...> wrote: >>> Since I finished the work on gen.db, the old dbdir is no longer used >>> and should be removed. All that logic is now in base-read-write with >>> supporting modules for transactions, cursors etc. >>> >>> So we could add new functions to base, read and write if desired. >> >> base is the only one of these that doesn't have bsddb specifics in it. >> Brian is suggesting hierarchy for read and write that would be below >> bsddb stuff. So, GrampsDbRead should be named BsdGrdbRead or similar >> and extend this new generic read class. >> >>> Let's be careful though to only add truly general functions that will >>> be used by many modules, not just one or two. >> >> The problem with using the criteria of "not general enough" is that >> these little functions pop up in places like the RelationshipView. Now >> that we are adding this kind of functionality in different places >> (like new views, cli, gramps-connect), the functions get replicated. >> Now we have multiple functions that do the same thing, and some might >> not be as well written as others. >> >> I think a better criteria for these functions is "where does it >> belong?" I think a rule of thumb should be: if you are manipulating >> handles, then ask yourself: should this function be in the database? >> If it doesn't belong in the database, should it be in a collection of >> accessor functions? >> >>> Also, if we add functions to gen.db we have to do it again in the >>> proxies. >> >> I think the idea that Proxies would extend their respective bases >> (read or write). >> >> -Doug >> >>> Perhaps we can group the new functions in a separate module >>> and mix them in as needed at run time. Just a thought. >>> >>> On 12/21/09, Brian Matherly <br...@gr...> wrote: >>>> Doug, >>>> >>>>> Finally, a question. Brian and I were just having a >>>>> discussion about >>>>> where to put "business logic" code. For example, consider: >>>>> >>>>> def marriage_from_eventref_list(db, >>>>> eventref_list): >>>>> """ >>>>> Get the marriage event from an >>>>> eventref list. >>>>> """ >>>>> for eventref in eventref_list: >>>>> event = >>>>> db.get_event_from_handle(eventref.ref) >>>>> if event and >>>>> event.type.is_marriage(): >>>>> >>>>> return event >>>>> return None >>>>> >>>>> This type of function has previously ended up in places >>>>> like >>>>> ReportBase.ReportUtils or gen.utils.dbutils depending on >>>>> the functions >>>>> perceived utility. I've put some of these functions as >>>>> methods on >>>>> gen.db.base.py (by changing db -> self). Brian has >>>>> pointed out that >>>>> base.py is a virtual class only. Furthermore, Brian says >>>>> business >>>>> logic does not belong in the database, and cited the >>>>> Single >>>>> Responsibility Principle [1]. However, to me these look >>>>> like Db >>>>> methods. If they are in the Db, then it makes it very easy >>>>> to find >>>>> them and use them (even from Django). Some of these methods >>>>> might >>>>> alter the database, so they don't all belong in a read-only >>>>> proxy, and >>>>> thus agree base is not the right place for them. >>>>> >>>>> The question: >>>>> >>>>> 1) should such methods be put in a Db class, say, >>>>> right above >>>>> GrampsDbRead so that GrampsDb and Django can share them? >>>>> Pros: they >>>>> are all in one place. Cons: this code does all kinds of >>>>> logic, >>>>> breaking [1]. >>>>> 2) or should they be put in a module and used as functions? >>>>> This does >>>>> keep them separate, but you have to find them based on how >>>>> there are >>>>> categorized. >>>> >>>> Depending on which functions you are talking about, I think most of them >>>> can >>>> go in the inheritance chain somewhere between GrampsDbBase (base.py) and >>>> GrampsDBDir (write.py). In order to support this, I think we should >>>> update >>>> some of the database classes. Since we will have two implementations >>>> (django >>>> and bsddb), we should be sure to give the implementation names >>>> appropriate >>>> qualifiers (like BsddbGrDb and DjangoGrDb). Here is my suggestion: >>>> >>>> ======= RoGrDb ======== >>>> First, let's have an abstract class that only contains the read >>>> functions. >>>> This class should live in rogrdb.py and should only have "get" functions >>>> that raise NotImplemented errors. This base will be inherited by RwGrDb >>>> and >>>> all the proxies (which are, by definition, read only) >>>> >>>> location: gen.db.rogrdb >>>> >>>> ======= RwGrDb ======== >>>> Next, let's have a (mostly) abstract class that adds the "write" >>>> functions >>>> to the database interface. All "set", "add" and "remove" functions should >>>> raise NotImplemented errors - except for functions that are database >>>> agnostic (set_birth_death_index, remove_parent_from_family, >>>> remove_family_relationships, delete_person_from_database, >>>> remove_child_from_family, add_child_to_family) >>>> >>>> RwGrDb should inherit from RoGrDb >>>> >>>> location: gen.db.rwgrdb >>>> >>>> ======= BsddbGrDb ======== >>>> This would be the existing GrampsDbDir implementation. >>>> >>>> BsddbGrDb should inherit from RwGrDb and implement all of the abstract >>>> functions. >>>> >>>> location: gen.db.bsddbgrdb >>>> (must go in gen because it is shared by gui and cli) >>>> >>>> >>>> ======= DjangoGrdb ======== >>>> This would be the existing DjangoDb implementation. >>>> >>>> DjangoGrdb should inherit from RwGrDb and implement all of the abstract >>>> functions. >>>> >>>> location: web.djangoGrdb >>>> (may go in web because it is not shared) >>>> >>>> .... >>>> >>>> This is all just a suggestion. But it makes sense in my mind (I'm >>>> certainly >>>> not the database expert, and I hope Benny and some others chime in). >>>> >>>> Also, I used GrDb as the suffix in the examples above. I don't have a >>>> strong >>>> preference for prefix or suffix names for all the database classes - but >>>> I >>>> do think that we should come up with a convention and then stick with it >>>> consistently. >>>> >>>> >>>> ====== Regarding marriage_from_eventref_list() specifically ===== >>>> You can apply the same logic to this function as the other db agnostic >>>> functions I listed above. However, this is a fairly obscure function. It >>>> is >>>> only used in familymodel.py for quickly building columns from directly >>>> unpickled data. As such, I think it is specific enough that it could just >>>> be >>>> moved to familymodel.py. But its up to you. >>>> >>>> ... >>>> >>>> That's all for now. Thanks for reading. I Hope this encourages some >>>> discussion. >>>> >>>> ~Brian >>>> >>>> ------------------------------------------------------------------------------ >>>> This SF.Net email is sponsored by the Verizon Developer Community >>>> Take advantage of Verizon's best-in-class app development support >>>> A streamlined, 14 day to market process makes app distribution fast and >>>> easy >>>> Join now and get one step closer to millions of Verizon customers >>>> http://p.sf.net/sfu/verizon-dev2dev >>>> _______________________________________________ >>>> Gramps-devel mailing list >>>> Gra...@li... >>>> https://lists.sourceforge.net/lists/listinfo/gramps-devel >>>> >>> >>> -- >>> Sent from my mobile device >>> >>> Gerald Britton >>> >>> ------------------------------------------------------------------------------ >>> This SF.Net email is sponsored by the Verizon Developer Community >>> Take advantage of Verizon's best-in-class app development support >>> A streamlined, 14 day to market process makes app distribution fast and >>> easy >>> Join now and get one step closer to millions of Verizon customers >>> http://p.sf.net/sfu/verizon-dev2dev >>> _______________________________________________ >>> Gramps-devel mailing list >>> Gra...@li... >>> https://lists.sourceforge.net/lists/listinfo/gramps-devel >>> >> > > -- > Sent from my mobile device > > Gerald Britton > |