JOAQUIN M. LOPEZ MUÑOZ skrev:
I've tried to address most of the suggestions and comments raised during the review. The two most important changes are: 1. Alberto Barbati's insightful review has led to the introduction of so-called key-value flyweights (http://tinyurl.com/5mnwjp ). This type of flyweights is meant for the situations where constructing the underlying type is expensive and must be avoided except if absolutely necessary. I think the design is robust enough and covers the variants (inner/outer key) that arose during the discussion with Alberto.
I had a preliminary look at the key/value stuff of the revised flyweight library. I assume that the rest has not undergone severe modifications so I concentrated myself on that part. First of all, let me congratulate with the good job. It really is a big improvement in addressing a scenario I faced (too) many times in the past. Here's my first thoughts about the new feature: 1) I like the idea of having a flyweight<key_value<K,V>> rather than flyweight<K,V>. It is more consistent with the rest of the framework 2) It's good to have an optional key extractor 3) You assume that Value is constructible from Key. This initially stroke me as a big restriction, but in fact in case you have a class that doesn't have a suitable constructor, you can always wrap it. For example: class ThirdPartyTextureClass // I can't modify this class { public: ThirdPartyTextureClass(const char* filename, /* other params here */); /* ... */ }; // can't have flyweight<key_value<std::string, ThirdPartyTextureClass> > struct texture_wrap : ThirdPartyTextureClass { texture_wrap(const std::string& filename) : ThirdPartyTextureClass(filename.c_str(), /* ... */) {} }; // now this is good: flyweight<key_value<std::string, texture_wrap> > f; I believe this might be mentioned in the docs, if you have time for that. Another approach that would make sense is to add a "factory functor" argument in addition to the "key extractor". This would help in cases where derivation is impossible or would look odd, for example if the value type is a basic type or a smart pointer. 4) I don't understand why you need Value to be Assignable. I believe it's really important that you could support non-Copiable and non-Assignable value types. The main reason for using flyweight is precisely to avoid having more copies of the value than the strictly necessary. If you are going to copy values, you are failing this requirement. 5) (probably related with point 4) I don't understand the role of copy_value() HTH, Ganesh