[boost] Re: Re: Re: [review] Formal review of the SmartContainerlibrarystarts today

From: Thorsten Ottosen (nesotto_at_[hidden])
Date: 2004-10-05 02:30:09

| > | >
| > | > | 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.
| >
| > how often do we need the nulls? (ie, would you need it for all classes?)
|
| I can't make an estimate (which would only be subjective), but I think
| there are cases.
|
| Another problem, IMO, is that client code will need to be more
| complicated, in order to find out if a value is null or not.

I don't understand that. One of the objectives of a null object is that you
don't have to
worry about a value being null.

| >
| > | Besides, there are operations which don't make sense for a "null
object".
| >
| > can you give some examples?
|
| How about, cloning ;) ?

struct null_object : public object
{
    null_object* clone() const { return new null_object(*this); }
};

| >
| > | And, for each new (virtual) function you add, you should update the
| > | null_object class - which I think can turn into a maintenance nightmare
:(
| >
| > if your inheritance hierarchy contains N distinct classes, you would need
to
| > add N implementations of that virtual function anyway, right?
|
| Not necessary... I can add one implementation, and some of those
| distinct N classes don't have to override it ;)

you could insert the null object class between the abstract base class and the
leaf classes.

| > | I don't understand the above phrase.
| >
| > if you want to extend the std::map for some reason or make your own map,
then
| > you can do so and you can use that new map as a basis for a smart
container
| > too using
| > ptr_map_adapter.
|
| That is quite ok. But please rephrase the example or so, because from
| the example, tihs was quite unclear (to me).

yes, will do.

| I'm not sure what you mean, but I think it would be worth it.
| Again, I find it very misleading for CloneManager::operator() to do
| destruction.

ok, noted.

| > | >
| > | > ok, also if it means loss of optimality in for map/set with uinque
keys?
| > |
| > | Would you please exemplify? Thanks.
| >
| > for example, consider
| >
| > boost::ptr_set<T> set;
| > boost::ptr_vector<T> vec;
| > ...
| > // fill vec with many duplicates as defined by operator< on T
| >
| > for( 0..vec.size() - 1 )
| > {
| >     // #1
| >     set.insert( vec[i] );
| >     // #2
| >     set.insert( allocate_clone( vec[i] ) );
| > }
| >
| >  #1 can check if vec[i] already exists as defined by operator< on T.
| >  If it does exists, there is no need to make a new clone. #2 always make a
new
| > clone
| >  and might need to delete it afterwards
|
| I might be totally off-base here, but why not an insert_clone function?

That would be more explicit.

It might be worth to have the existing interface if users want to try changing
from vector<string> to ptr_vector<string>
just to see what happens.

br

Thorsten