Thorsten Ottosen wrote:
Hi John,
Thanks for the review.
"John Torjo" <john.lists@torjo.com> wrote in message news:415EF071.3040204@torjo.com... | | > | > * What is your evaluation of the design? | | The design is ok. I'm still wondering if we really need two pairs of | iterators (begin()/end() and ptr_begin()/ptr_end()).
so which would you remove?
ptr_begin and ptr_end.
| Also, I don't like the fact that you can't have null pointers but I can | live with that ;)
ok, do you find the null object pattern undesirable? I actually think it is a strength.
There are times when it's a good thing (the null object pattern). But I'm really against creating such a class for each type I want to use smart_container for. Besides, there are operations which don't make sense for a "null object". And, for each new (virtual) function you add, you should update the null_object class - which I think can turn into a maintenance nightmare :(
| --------------------- | Changing the Clone Manager | | You should first explain some of the internals, before jumping to "Clone | Managers" and such.
ah yes, the tutorial can be much better...and will be eventually.
:) look forward to it ;)
| ---------------------- | Extending a map | | I don't understand the need for my_map class which just derives from | std::map. | Why not use std::map directly?
It's beyond what I want to do myself, but the idea should be that you can if you want.
I don't understand the above phrase.
| Relative to this: | // | // add constructors and assignment operators here, none needs | // to be defined | // | | What do you mean - they don't need to be defined? If I add a constructor | or so, I should define it somewhere. Otherwise, what's the point of | declaring it?
the point is that in this approach you don't need to define more than you want; the example above requires you to define standard map contructors because the ptr_map code depends on those constructors.
Well then, say so ;) Something like "add constructors here (optional)" - or something.
| ------------------------- | Clone manager requirements | | | "the Clone Manager need not be stateless, but the assumption is that it | probably will be." | Which is it? Can it be stateless or not?
it can be and the two default ones are. I need to express this clearer.
yup.
| "Ownership can be transferred from a container to another container on a | per iterator range basis" | I wonder if a bettern name for the transfer function would be "move()".
Move sounds ok. It might be confused with move semantics though.
But it seems to be move semantics ;)
| | | ---------------------------- | Class reversible_smart_container | | "All objects must be deallocable with CloneManager::operator()" | | This seems a very "smart" way of deallocating objects. Is this a typo?
not sure what you mean. operator()() is from the clone manager is used as a deleter all over the place, internally, that is were the requirement stems from.
Yes, but for me - by reading CloneManager::operator(), I would think it would provide a clone. Thus, it's misleading. I think it would me more straightforward not to use operator() in this case. You could have a function, called 'destroy()' or something.
| 9. #Should we add iterators for traversing the keys of a map? This would | mean we could say | | map::key_iterator b = m.begin(), e = m.end(); | copy( b, e, some_where ); | | | | I would not like that very much. | As a side note, Boost Rangelib (formely know as Range Template Library | - www.torjo.com/rangelib/) already provides means to walk through | the keys or values of a collection.
ok, how does it look like?
Like this: std::map<key,value> m; std::vector<key> k; std::vector<value> v; rng::copy( transformed(pair_1st(m)), std::back_inserter(k) ); rng::copy( transformed(pair_2nd(m)), std::back_inserter(v) );
| 12. #should we provide member algorithms for unsafe standard algorithms | like unique() and remove? | | Why would you say "unsafe"? | Maybe I'm overlooking something, but std::unique/std::remove should work | fine now.
depends on what iterator you use. when using pointers these algorithms can remove pointers and duplicate others in the containers.
oh yes! I see the light now ;) You're totally right.
| ------------------------- | FAQ | | Why are inserting member functions overloaded for both pointers and | references? | REVIEW QUESTION: do we want this behavior? | | No.
ok, also if it means loss of optimality in for map/set with uinque keys?
Would you please exemplify? Thanks. Best, John -- John Torjo -- john@torjo.com Contributing editor, C/C++ Users Journal -- "Win32 GUI Generics" -- generics & GUI do mix, after all -- http://www.torjo.com/win32gui/ -- v1.4 - save_dlg - true binding of your data to UI controls! + easily add validation rules (win32gui/examples/smart_dlg)