Re: [Gramps-devel] Displayer Strategy

[Doug B. <dou...@gm...>]

On Mon, Jan 11, 2010 at 10:20 AM, Benny Malengier <ben...@gm...
> wrote:

> 2010/1/11 Doug Blank <dou...@gm...>:
> > On Mon, Jan 11, 2010 at 8:37 AM, Brian Matherly <
> br...@gr...>
> > wrote:
> >>
> >> Benny,
> >>
> >> > > As part of GEP008, I'm trying to figure out what to do
> >> > with the DateDisplay and NameDisplay classes.
> >> > >
> >> > > Here is my suggestion:
> >> > >
> >> > > 1) Create a new directory in gen called "display".
> >> > This directory will be for code that facilitates the
> >> > formatting of data for display (like DateDisplay and
> >> > NameDisplay)
> >> > >
> >> > > Initially, we will have gen.display.date and
> >> > gen.display.name. But I can imagine that eventually we will
> >> > have displayers for other objects. I already know of some
> >> > code in a random "utils" file somewhere that formats Places
> >> > for display. That code could ultimately find a home in
> >> > gen.display.place.
> >> > >
> >> > > 2) Create a DisplayerFactory class which can do the
> >> > work of figuring out the right displayer to use. Right now,
> >> > most code accesses the global BasicUtils.name_displayer and
> >> > DateHandler.displayer objects. This is bad programming
> >> > practice. With the new factories, you can call:
> >> > >   gen.display.factory.get_displayer("Date")
> >> > > and the factory will make sure that you get the right
> >> > displayer for the current locale and user preferences.
> >> > > Similarly, calling:
> >> > >   gen.display.factory.get_displayer("Name")
> >> > > would return a NameDisplay object properly initialized
> >> > to the current user settings.
> >> > > This way, the Displayer classes themselves would not
> >> > depend on config and we could remove code like this:
> >> > >        global WITH_GRAMP_CONFIG
> >> > >        self.name_formats = {}
> >> > >
> >> >  self.set_name_format(self.STANDARD_FORMATS)
> >> > >
> >> > >        if WITH_GRAMPS_CONFIG:
> >> > >            self.default_format =
> >> > config.get('preferences.name-format')
> >> > >            if self.default_format == 0:
> >> > >                self.default_format =
> >> > Name.LNFN
> >> > >
> >> >  config.set('preferences.name-format',
> >> > self.default_format)
> >> > >        else:
> >> > >            self.default_format = 1
> >> > >
> >> > > I would appreciate you comments. If I can move forward
> >> > with this scheme, I'll add it to the GEP008 wiki page.
> >> >
> >> > If I understand correctly, factory will depend on config,
> >> > and call
> >> > displayer with correct settings?
> >>
> >> Yes. Displayer will not have any dependency on config.
> >>
> >> > For me your suggestion sound ok. The idea of Doug with str
> >> > is nice,
> >> > but what we do is a bit more versatile than just giving a
> >> > nice str
> >> > reprensation of an object, so I would stick with specific
> >> > display
> >> > classes, with str in the object being more low level.
> >>
> >> Doug's idea would make gen.lib bloated because name.py and date.py would
> >> have all the logic for displaying and they would have to depend on
> config
> >> (they might already, I don't know). Date.py would have to know about the
> >> plugin system to choose the correct localized date displayer. So it
> would
> >> make those already large classes even larger.
> >>
> >
> > Brian,
> >
> > I understand your perspective, but let's not describe one as "bloating up
> > the code" and the other as not. For example, in your methodology, you
> will
> > need to do this in *every* place you want to display a name and a date:
> >
> > from someplace-that-i'll-always-have-to-lookup import some namedisplayer
> > from someplace-that-i'll-always-have-to-lookup import some datedisplayer
> > from some-other-place-maybe import perhaps-do-some-factory-weirdness
> > nd = perhaps-do-some-factory-weirdness(namedisplayer)
> > dd = perhaps-do-some-factory-weirdness(datedisplayer)
> > name_text = nd(person.get_primary_name())
> > date_text = nd(person.get_birth_date())
> > print "%s was born on %s" % (name_text, date_text)
> >
> > That, in comparison to:
> >
> > print "%s was born on %s" % (person, person.get_birth_date())
> >
> > I agree that what I suggested would put this boilerplate code in those
> two
> > classes, and perhaps add a dependency, but it would be done in one place,
> > thus reducing overall bloat.
> >
> > Something is going to have a dependency on config. Why are you jumping
> > through hoops to not to put this in the most obvious places
> > (gen.lib.Name.__str__)?
>
> I also don't think that our gen.lib should contain import from utils,
> it just makes for better coding.
> But ease of use is also important.


Ah, I see where you both are coming from. My philosophy is "ease of use
*makes* better coding". Related to the idea that code is not to be
replicated (Don't Repeat Yourself, DRY
http://en.wikipedia.org/wiki/Don%27t_repeat_yourself).

One could always inject the
> dependency in the class, so bind a class function that is called on
> str.
>
> The main issue is that you need to be able to switch display between
> certain plugins, so this calls for not a single str, you would need an
> option to override the default then. After all, you would in a plugin
> indicate you want name display as A, while the person view still has
> to show B.
>

That's a good point for that use case (which is rare, and doesn't need to be
the default behavior).


> This calls for a setup that is a bit more versatile than just str.
> Date is more complicated, it calls for loading localized datehandlers,
> which are plugins, there is no need for an str of an object to do
> that.
>
>
Ok, then I amend my suggestion to have __str__ as the default, and have a
gen.lib.Name.display method that takes parameters.


> Anyway, the code would look like
>
> import gen
> from gen.lib.date import Date
> _nd = gen.display.factory.get_displayer("Date")
> print _nd(gen.lib.date.Date(0))
>
> If you need to override the default probably an extra argument. We
> could make a convenience function to make it shorter and easier to
> remember
>
> from gen.display import displaydate
> from gen.lib.date import Date
> print displaydate(Date(0))
>
> Which to override the default, would also need an optional extra
> argument, eg the language code.
>
>
I think this is very similar to the gen.lib issues that we have with
different backends. I think a connected data Engine would be the right place
to do this.

-Doug


> Benny
>