Re: [boost] [flyweight] Review period extended to February 3

From: Joaquín Mª López Muñoz (joaquin_at_[hidden])
Date: 2008-02-07 13:07:44

Alberto Ganesh Barbati ha escrito:

> Joaquín Mª López Muñoz ha scritto:

[...]

> > How do I know if value_type is convertible to key_type? I need asistance
> > from the user.
> >
>
> The user might be allowed to provide that as an additional extension
> point. However, calling the value_type constructor with the key_type as
> parameter is a reasonable default behaviour.

I was referring to the reverse process, from value_type to key_type,
looks to me you're talking about key_type-->value_type here.

As for the construction key_type-->value_type, I agree with you calling
value_type ctor is a resonable default, but this should be replaceable
by the user (for instance, keyed_flyweight<K,T*> typically will need a
function calling new T rather than a pointer's ctor).

[...]

> > Well, this is a valid approach, but I'd rather factories remained dumber so
> > that all this intelligence is implemented in the core, because of the following
> > reason (among others): STL associative containers (which are the natural
> > components upon which to implement factories) are either set-like
> > (key_type=value_type) or map-like (key_type is a fixed part of value_type,
> > which does not coincide with user's provided T). To be able to accept both
> > the keyed and non-keyed variants, the container would have to deal with a more
> > general notion of key extraction. Boost.MultiIndex can do that, but I don't
> > want to tie flyweight to that lib.
>
> If you stick to that, you really should consider a name different from
> factories, which indeed are nothing more than storage.

I don't mind doing that (despite the tradition "factory" has in the expositions of
this pattern) if there's an agreement about the name. Some reviewer proposed
"pool", this does not look bad at all to me.

> Anyway, I get your point. The design I have in mind is a bit different, though,
> leaving more intelligence to the factory. Even in your design, std::set can't be used
> directly, it's been wrapped in set_factory<>. So what's wrong in having this added
> intelligence put inside the wrapper, rather than into the core?

Well, although containers themselves are not directly usable as factories, as
you point out, the work one needs to do in order to wrap it into an appropriate
factory is trivial, and this is how I'd like to keep it.

The following is a design rationale and you might legitimately disagree, but my
goal is to decompose the fylweight ocnfiguration into a number of aspects that
are:

  1. Orthogonal
  2. Simple
  3. Natural

The idea is that the core provides the extra intelligence needed to glue the
components together. Hence my reluctance to add intelligence to the factories
or fuse tracking or locking with factories.

> About the "third" case, you don't need to tie Boost.MultiIndex with the
> core. The library would simply propose (among several others) one
> factory implemented as a wrapper onto Boost.MultiIndex. Only user that
> needs that feature would need to Boost.MultiIndex. This would be no
> different than the current design, where hashed_factory depends on
> Boost.MultiIndex and let's not forget that hashed_factory is the default
> factory!

My point is not that B.MI should be avoided, bu that it *can* be avoided:
mandating that factories support in-place key_value's (i.e. some sort of
key extraction from value_type) implies that you're going to have to use
sophisticated data structures to implement your own factories --confront
that with the fact that set_factory can de build in a couple dozen lines
from a std::set.

> > On the other hand, I don't know how to solve the third case (T convertible
> > to K) with dumb factories :-( Maybe this third case is not worth implementing,
> > given that the external key variant shouldn't require that much extra storage
> > (shared values ought to be comparatively few with respect to the number
> > of flyweight objects pointing to them.)
> >
> > Maybe I can provide an external flyweight wrapping both behaviors
> >
> >   flyweight<T,...> // classical flyweight
> >   flyweight<T,key<K>,...> keyed flyweight
> >
> > or something to that effect. I definitely have to think this carefully.
> >
>
> If I could rethink the design from scratch (and I know I can't as the
> library has already been accepted)

AFAICS nothing is still cast in stone: we enjoy design freedom until the first
release of Boost containing Boost.Flyweight, which won't happen in
a number of months. So, do not hesitate to contribute your opinions!

> I would probably come up with something like:
>
> 1) the core class flyweight with has only two parameters: the factory
> and the holder policy
>
> 2) a bunch of different factories implementations. In particular there
> should be at least three factories to cover the three basic cases: K==T,
> K!=T non-intrusive and K!=T intrusive. These factories may be
> parametrized with a user-provided container or a separate parametrizable
> factory should be provided ad hoc. Most important, all library-provided
> factories shall be parametrized by the locking and tracking policy
>
> 3) a bunch of holder, locking, tracking policy implementations
>
> The choice to have locking and tracking parametrize the factories
> instead of the core is that actually no locking nor tracking happens in
> the core!

[no tracking happens at the factory either]

> Locking happens only in the execution of insert() and erase(),
> which would be moved in the factory, and tracking can be obtained as a
> RAII wrapper provided by the factory (I admit that I didn't thought
> about tracking deeply enough to pretend that this is actually possible,
> but I'm sure about locking).

I think that tracking performs adequately as it is, given that no interaction
problem have been detected. As for locking, I'd prefer that it be managed by the
core for the reasons explained above, but I'm not totally opposed to te
alternative you propose if that proves to be the most efficient design. I need
to do some prototyping before taking a decision.

One final note: if we analyze this paragraph of yours:

"a bunch of different factories implementations. In particular there
should be at least three factories to cover the three basic cases: K==T,
K!=T non-intrusive and K!=T intrusive. These factories may be
parametrized with a user-provided container or a separate parametrizable
factory should be provided ad hoc. Most important, all library-provided
factories shall be parametrized by the locking and tracking policy"

and do the following replacement

  "factory"->"core"
  "container" -> "factory"

we've got this:

"a bunch of different core implementations. In particular there
should be at least three cores to cover the three basic cases: K==T,
K!=T non-intrusive and K!=T intrusive. These cores may be
parametrized with a user-provided factory or a separate parametrizable
core should be provided ad hoc. Most important, all library-provided
cores shall be parametrized by the locking and tracking policy"

which is disturbingly similar to the approach I'm favoring... Maybe we're
disagreeing just in names after all?

Joaquín M López Muñoz
Telefónica, Investigación y Desarrollo