[merge] LazyFactory
Adeodato Simó
dato at net.com.org.es
Tue Aug 15 03:14:47 BST 2006
* John Arbash Meinel [Mon, 14 Aug 2006 11:34:32 -0500]:
> > + def register(self, key, object):
> > + """Register a new object to a name.
> > +
> > + :param key: This is the key to use to request the object later.
> > + :param object: The object to register.
> > + """
> > + if self._first_is_default and not self._dict:
> > + self._default_key = key
> > + self._dict[key] = object
> If you are going to have an explicit _default_key object, then this
> should be:
> if self._first_is_default and self._default_key is None:
> self._default_key = key
Well, while I see your point, I think bool(self._dict) reads closer to
"is this the first registered entry?", than checking _default_key. But
if you're strong on having it with "_default_key is None", I don't
object at all to changing it.
> Sending 2 bundles makes it a little confusing what should be reviewed. I
> realize you are just being thorough. Maybe send the rollup as a
> patch.gz, so that it doesn't show up, but can be used to merge from.
Okay, sorry for that.
> What does Registry provide over a plain dictionary?
A way to implement the "register_function/get_function/list_available"
pattern in a consistent way all over bzrlib (modules export just a
Registry object instead of their pair of functions). Also, with
standardization, stuff can be built on top of it, like FromRegistryOption.
> v- I don't really prefer being able to pass in what the default key is
> as part of 'get()'. If you really want that, then it should be done in
> another location. (Say at 'init' time). Looking over it, you have about
> 3 different ways to specify what the default key is. I think we would
> want only 1. I could see 'get()' having a 'default_value' parameter, in
> the case that a key didn't exist. But not a 'default_key' parameter.
> > +
> > + def get(self, key=_marker, default_key=_marker):
> > + """Return the object register()'ed by the given key.
> > +
> > + This may raise KeyError if the key is not present.
> > +
> > + :param key: The key to obtain the object for; if not given, :param
> > + default_key: will be used.
> > + :param default_key: Key to use if an object for :param key: can't be
> > + found; defaults to self.default_key. Set it to None if you'd like
> > + to ensure an exception is raised for non-found keys.
> > + :return: The previously registered object.
> > + """
> > + if default_key is _marker:
> > + default_key = self.default_key
> > +
> > + if key is _marker:
> > + return self._dict[default_key]
> > + else:
> > + try:
> > + return self._dict[key]
> > + except KeyError:
> > + return self._dict[default_key]
> > +
On perspective, the name of the parameter should probably be "fallback_key"
instead of "default_key". I've changed that in my branch.
The intent is to supply the functionality of default_value, but assuming
such value would always come from the registry itself, so that instead
of e.g.:
foo = registry.get(user_supplied, registry.get(another_one))
you could do:
foo = registry.get(user_supplied, another_one)
> I would rather the default always be key=None, because then clients have
> a chance to pass that in. So calling code is:
> key = <get some key from the user in some fashion>
> obj = registry.get(key)
> Otherwise, callers have to do:
> key = <get some key from the user in some fashion>
> if key is None:
> obj = registry.get()
> else:
> obj = registry.get(key)
Oh, good point. I've fixed that (diff included below; mergeable from [1]).
[1] http://people.debian.org/~adeodato/code/branches/bzr/registry_class
> Anyway, I'm happy with the idea of creating a different base class, as
> long as it does something useful. I don't know that having lots of
> different ways to specify a default is useful. I suppose having the
> abstraction lets you swap out one Registry type for another if you want
> different functionality later on. (Like failing if a key already exists,
> or returning the original value from the register() function).
Nice. So to clear things up, this should be ready to enter after you
tell me your preference about this two issues:
- the register() bit mentioned at the beginning of this mail
- whether to drop fallback_key in get(), and whether to replace it
with default_value=None (in which way, as a side-effect, the method
would never raise a KeyError exception, but return None instead).
I hope this can make it into 0.10, maybe even with making log.py use it.
BTW, would this addition need a NEWS entry under "INTERNALS"?
Thanks,
--
Adeodato Simó dato at net.com.org.es
Debian Developer adeodato at debian.org
Listening to: Los Planetas - Pesadilla en el parque de atracciones
More information about the bazaar
mailing list