Hi Alex, it turns out that the itch is real, and this is actually going to be useful in carrying forward the HttpHeader restructuring I'm doing in lp:~kinkie/squid/coverity-fixes, to iterate over the list of header types for various purposes (compacting, etc). So I went ahead and just implemented it, starting from your observations. It's a bit more complex that your proposal, as I took the chance to implement bidirectional forward and reverse iterators. The attached patch is in changeset 14235 of lp:~kinkie/squid/coverity-fixes, but since the code is quite well self-contained I'd like to submit it for audit as a standalone unit; If it passed I'll merge it to trunk with the code that uses it.
Thanks! On 08/05/2015 07:58 PM, Kinkie wrote: > What I understand from the changes you propose is that: > 1. instead of having explicit begin/end markers passed as template > arguments you propose to have a convention on begin/end markers. There are several different things/layers mixed up here: * If we want to use range loops for enums [in a sane way], then our enums should have begin/end markers. Many already do, but we will have to standardize their names and fill the gaps. * The begin/end parameters should not be template parameters. * If we need to iterate sub-ranges, then we should add EnumRangeT<Enum> class with begin/end as constructor parameters (see below). > 2. you skip the typedef fixing these parameters in favor of letting the > template parameters appear in the calls. There are two separate issues/layers here: * Naming an expression is a good idea, especially if that name is going to be used multiple times. * Fewer template parameters are generally better if they can accomplish the same thing as more template parameters. > 1. looks interesting, especially as an enum missing those markers will > fail to compile. Maybe both approaches can be combined by using default > template arguments, what do you think? > template <typename C, C first = C::enumBegin_, C last_plus_one = > C::enumEnd_> I think we should not specify iteration range boundaries as template parameters. > 3. we could maybe combine the benefits of beautiful range-for and ugly > explicit-for also for shorter ranges by having (in the class-based > variant), on top of what you have already proposed (untested) Yes, if sub-range iteration using range loops is needed, then we should add EnumRangeT<Enum> class that does it. Sorry if I have not mentioned that explicitly. Your sketch for that class is fine, but it will be a EnumRangeT class, separate from simpler WholeEnum, and we would try to define a function that will create that EnumRangeT object so that we only have to type "Numbers" zero or two times as illustrated below: template <typename Enum> inline EnumRangeT<Enum> EnumRange(const Enum from, const Enum to) { return EnumRangeT<Enum>(from, to); } So that instead of: > for (auto i : WholeEnum<Numbers>(Numbers::one, Numbers::three)) { > // do stuff > } we could do: // iterating sub-range of a class enum: for (auto i: EnumRange(Numbers::one, Numbers::three)) ... // iterating sub-range of a global C enum: for (auto i: EnumRange(one, three)) ... // iterating a whole enum: for (auto i: WholeEnum<Numbers>()) ... Again, the above range objects can be named (and the first two should be named if they are used more than once). For example: WholeEnum<Numbers> AllNumbers() { return WholeEnum<Numbers>(); } EnumRange<Numbers> FooBarNumbers() { return EnumRange(Numbers::foo, Numbers::bar); } > It looks to me that I have crossed an itch you feel as well, No, I am just trying to help you scratch your itch without scarring Squid code. I do not know whether your itch is real or imaginary. Cheers, Alex.
EnumIterator-v1.patch
Description: Binary data
_______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev