From: Marcus Lindblom (macke_at_[hidden])
Date: 2008-01-23 04:40:52
Ion Gaztañaga wrote:
> The formal review of Flyweight library, proposed by Joaquín M. López
> Muñoz, begins today (sorry for the late announcement):
> * What is your evaluation of the design?
Very nice. I think the various options are well thougt out and all.
However, I'd like to nit-pick some naming:
* The namespace should be boost::flyweight, without the s, since that's
what the lib & design pattern is called.
* The factory not only creates objects, it also stores them. When I see
'Factory' I think creation only, not storage. How about naming it the
'store' instead, since it seems to be the primary function?
Does the factory even create objects? It doesn't seem like that. It just
stores copies of them in Entrys?
Also, I think that a reader-writer lock would be good (as another poster
wrote), but that's not critical.
> * What is your evaluation of the implementation?
Not looked at thoroughly.
However, there is a dependency on boost::hash<>. Could this be changed
into conforming to a concept that just allows the value to be converted
> * What is your evaluation of the documentation?
However, on the reference page, the first header ("boost/flyweight.hpp")
doesn't link to anything?
Also, the flyweight.hpp page misses to mention the tag argument in the
first list of parametrization (above the code-block).
> * What is your evaluation of the potential usefulness of the library?
High. I can see a few places where this could be useful in our apps
> * Did you try to use the library? With what compiler?
> * How much effort did you put into your evaluation?
Quick reading. (1h)
> * Are you knowledgeable about the problem domain?
Haven't used flyweights, but am familiar with the problem.
> And finally, every review should answer this question:
> * Do you think the library should be accepted as a Boost library?
Yup! But some names ought to be changed to avoid confusion.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk