Re: [boost] [poly_collection] Andrzej's review

Subject: Re: [boost] [poly_collection] Andrzej's review
From: Andrzej Krzemienski (akrzemi1_at_[hidden])
Date: 2017-05-12 06:37:47

2017-05-12 8:13 GMT+02:00 Joaquin M López Muñoz via Boost <
boost_at_[hidden]>:

> El 12/05/2017 a las 1:07, Andrzej Krzemienski via Boost escribió:
>
>> Hi Everyone,
>>
>> Some comments from me about PolyCollection library, First, Joaquín, thank
>> you for writing and sharing this library.
>>
>
> Thank you for your review!
>
> [...]
>>
>> [1]
>>
>> Now, because I have no practical experience with the problem, what I say
>> in
>> this paragraph might be incorrect. But I have a problem here. You will use
>> this library for improved performance, maybe for a game implementation,
>> yet
>> because a game needs super-performance, can a programmer afford to use
>> dynamic dispatch, or RTTI or exceptions? I always imagined taht such
>> library, rather than resembling `ptr_vector<I>` would resemble
>> `vector<variant<A, B, C>>`, that is: I decide and fix the container on a
>> set of types I will be storing inside. This set of tyes is embedded in the
>> type, so I can detect many mis-usages at compile time. The implementation
>> can be faster, because the number of segments is fixed (no segment
>> management), and no registration checks need to be performed. Then,
>> iterate
>> over such collection with a static_visitor (from Boost.Variant). Of
>> course,
>> this would be constraining: I need to know all the types I will be using
>> ahead of time. But maybe in practice this is often the case?
>>
>
> I agree with you vector<variant<...>> should be faster than ptr_vector<I>,
> but this
> does not invalidate the lib rationale that replacing ptr_vector<I> with
> base_collection<I>
> will get you a speedup; of course the programmer has to decide on the
> various alternatives
> at their disposal, with different pros and cons. I don't know either if
> vector<variant<...>>
> is the "obvious" improvement over ptr_vector>I> to the extent that it
> deserves
> discussing/mentioning explicitly.
>

Just to make my remark clear, I am not saying one should use
vector<variant<A, B, C>> instead of ptr_vector<> for performance. Based on
my experience OO-inheritance solutions are faster that using `variant`. As
you say below, I was suggesting the alternative design:
`variant_collection`.

>
> On a related note, some have asked for a variant_collection to be part of
> Boost.PolyCollection roadmap.
>
> [...]
>>
>> [2]
>>
>> I am not comfortable with per-segment functions having the same name as
>> container-wide functions, and being only overloaded based on function or
>> template parameters, like `cc.size()` and `cc.size(index)`. These two
>> functions do different things, so they deserve different names. Maybe
>> `cc.segment_size(index)`, or `cc.serment(index).size()`?
>>
>
> On the contrary, I like name overloading better, because it looks terser
> yet sufficiently
> expressive:
>
>   cc.size<warrior>(); // what else but the numer of warriors in the
> collection?
>
> Furthermore, there are member functions such as
>
>   cc.erase(begin<warrior>());
>
> that, according to your suggestion, should be renamed as (IMHO very ugly)
>
>   cc.segment_erase(begin<warrior>());
>

or:

cc.segment<warrior>().clear();

Assuming that I understand what the former notation does.

> and if we decide that this latter example should be exempt from the
> segment_
> prefix, then what's the rationale for what gets segment_ and what does not?

I can see the problem you are describing. Also I agree with you that in the
examples above, using the overload is not ambiguous to the users.

On the other hand, I was encountering problems when trying to pass member
function names as parameters to to other functions. When function `size()`
is overloaded, I cannot easily pass its address to another function:

some_for_each(&T:: size); // ambiguous: which overload

But maybe it is just a bikeshed discussion. The library is good with either
choice of names.

Regards,
&rzej;