<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 6, 2015 at 12:38 AM, Alex Rousskov <span dir="ltr"><<a href="mailto:rousskov@measurement-factory.com" target="_blank">rousskov@measurement-factory.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>On 08/05/2015 09:33 AM, Kinkie wrote:<br>
<br>
> I crossed the topic of enumerating over iterations.<br>
<br>
</span>You mean iterating enum[eration]s :-)<br></blockquote><div><br></div><div>I have my thinking cap backwards ;)<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span>
> We kind-of follow (C-ish)best<br>
> practices on that, but that got me wondering if we can do better.<br>
> I came up with a trinket which on its face looks quite elegant to me,<br>
> and I'd like to understand with you guys if it makes sense to deploy it<br>
> in our code.<br>
<br>
</span>Maybe, after quite a few changes. It is risky for me to review this<br>
because the problems in the posted code lie on several very different<br>
layers, the code is outside Squid context, and I am not even sure what<br>
[CODE] thread designation means. I hope this does not lead to a long<br>
discussion, but here are a few things to consider:<br></blockquote><div><br></div><div>[CODE] means that it contains code which is not a [PATCH] but meant to spark a discussion which may or may not result into a patch.<br></div><div>I hope that the discussion will be fruitful, I don't mind if it's long as long as it doesn't waste people's time (read: it's fruitful)<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* Iterating enums under Squid control ought to be a lot easier, safer,<br>
and cheaper because we can add begin/end markers *with standard names*<br>
to each enum. This is important because most iterations iterate<br>
everything _and_ C++11 range loops always iterate everything. The<br>
"everything" case is worth focusing on! See attached files for examples<br>
of how standardized begin/end marks can be used.<br></blockquote><div><br></div><div>I agree. We'd need to take care to avoid the anti-pattern of "iterate over everything but not really everything", but that's exactly the itch I was trying to scratch, and found unsatisfactorily scratched in my online research.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* Your Iterator class is mixing two very different interfaces/APIs: One<br>
provides begin/end() methods and is required for range loops (I do not<br>
know how this API is called in the C++ standard). The other one is a<br>
[partial] iterator API. Do not mix those two APIs! Each of the attached<br>
files illustrates how to separate them (and there are two ways to<br>
implement the second/iterator API).<br></blockquote><div><br></div><div>Yes, I had unintentionally borrowed from Python where an object sometimes is its own iterator.<br></div><div>I don't have any particular attachment to the way I coded this - if that's what you are afraid of when you refer to the long discussion. Anything that is well self-contained and easy to use will fill the bill for me :)<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* The stand-alone iterator API implementation in<br>
t-enum-stand-alone-v3.cc allows for nice explicit for-loops and such,<br>
but those global operators are rather invasive, and I am worried they<br>
will cause problems in Squid context (as opposed a tiny test file!). I<br>
recommend that you start with t-enum-v3.cc and then diff the two<br>
attached files to see changes related to the stand-alone iterator API<br>
implementation. If you really like stand-alone operator side-effects,<br>
you may want to investigate whether they would work well in Squid context.<br></blockquote><div><br></div><div>Instinctivelty I would go for the class-based approach as it feels less invasive.<br></div><div>What I understand from the changes you propose is that:<br></div><div>1. instead of having explicit begin/end markers passed as template arguments you propose to have a convention on begin/end markers.<br></div><div>2. you skip the typedef fixing these parameters in favor of letting the template parameters appear in the calls. This is quite consistent with an approach often taken in the stdlib (e.g. std::numeric_limits).<br></div><div><br></div><div>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?<br></div><div>template <typename C, C first = C::enumBegin_, C last_plus_one = C::enumEnd_> <br></div><div>2. looks verbose to me, but it can be decided on a case-by-case basis, which can be a good or a bad thing.<br></div><div>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)<br></div><div><br>template <typename Enum><br></div><div>class WholeEnum<br>{<br></div><div>   Enum first, lastplusone;<br></div><div>public:<br></div><div>   WholeEnum (Enum firstValue, Enum lastValuePlusOne) : first(firstValue), lastplusone(lastValuePlusOne) {}<br></div><div>   Iterator begin() { return Iterator(first); }<br></div><div>   Iterator end() { return Iterator(lastplusone); }<br>}<br><br></div><div>this would allow to do<br></div><div>for (auto i : WholeEnum<Numbers>(Numbers::one, Numbers::three)) {<br></div><div> // do stuff<br>}<br><br></div><div>What do you think? Not very kosher maybe, but it should work.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
* Avoid static variables in templates, especially when you want to<br>
instantiate different templates based on such trivial things as range<br>
boundaries. This is (or used to be?) one of the dark C++ corners; they<br>
might hurt inlining/performance as well.<br></blockquote><div><br></div><div>Ok.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
The attached files are unpolished, but should illustrate the above<br>
points and how to avoid some of the problems discussed here.<br></blockquote><br></div><div class="gmail_quote">It looks to me that I have crossed an itch you feel as well, and this effort may be useful after all. <br></div>Thanks for taking the time!<br>-- <br><div>    Francesco</div>
</div></div>