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 <
> 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!
>> Now, because I have no practical experience with the problem, what I say
>> this paragraph might be incorrect. But I have a problem here. You will use
>> this library for improved performance, maybe for a game implementation,
>> 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,
>> over such collection with a static_visitor (from Boost.Variant). Of
>> 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
> 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
> is the "obvious" improvement over ptr_vector>I> to the extent that it
> 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:
> On a related note, some have asked for a variant_collection to be part of
> Boost.PolyCollection roadmap.
>> 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
> cc.size<warrior>(); // what else but the numer of warriors in the
> Furthermore, there are member functions such as
> that, according to your suggestion, should be renamed as (IMHO very ugly)
Assuming that I understand what the former notation does.
> and if we decide that this latter example should be exempt from the
> 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.