What do you think to this for loop macro?

Question

#define for_all_impl(var, cont, mode) for (auto var##_begin_it = cont.##mode##begin(), var##_end_it = cont.##mode##end(), var##_it = var##_begin_it; var##_it != var##_end_it; ++var##_it) if (bool b = true) for (auto& var(*var##_it); b; b = false, var) if (bool b = true) for (const auto var##_num(var##_it - var##_begin_it); b; b = false, var##_num)
#define for_all(var, cont) for_all_impl(var, cont, )
#define for_all_const(var, cont) for_all_impl(var, cont, c)
#define for_all_reverse(var, cont) for_all_impl(var, cont, r)
#define for_all_const_reverse(var, cont) for_all_impl(var, cont, cr)

int main()
{
    std::vector<int> a; a.push_back(0); a.push_back(1); a.push_back(2); a.push_back(3);
    for_all(i, a)
    {
        std::cout << i_num << ": " << i << std::endl;
    }
}

The reason I made it is because for a long time C++ had no easy shortcut to iterate through a data structure without lots of typing. Now we have the new range-based for loop syntax, but that has no easy way to give you the current index or go backwards (that I know of).

I spent a lot of time designing it so please give it the benefit of the doubt, and point out any big flaws. I have made the assumption only regular variable names will be used as parameters. I'm not putting it forward for everyone but I will use it in my project from now on unless it has a big problem.

Thanks.

Edit: I realise the index counter (i_num) won't work for all data structures such as lists. But I still think it's handy.

It would be easy enough to make wrapper objects/functions for use with range-based `for` to do this without macros. — Seth Carnegie, May 10 '13 at 22:43
Please do suggest an alternative. Will the final use be as simple? Will it optimize well? — Neil Kirk, May 10 '13 at 22:44
It scares me... I have no clue what it does. The impl macro is very long and it's hard to understand what's going on. — olevegard, May 10 '13 at 22:44
The only thing that matters is the use of the macro is simple! Right!? ;) — Neil Kirk, May 10 '13 at 22:46
If you forget about i_num, then using for_each with a lamba is nearly as simple. — Pete Fordham, May 10 '13 at 22:48
What would it take to convince you that this handy macro is an unmaintainable monstrosity? — Beta, May 10 '13 at 22:49
How difficult is it to write a few more characters (for a range based for loop) and rest assured that it works correctly (because it is not derived from macros) — A. K., May 10 '13 at 22:49
But I want to go backwards and know the index of the current variable. — Neil Kirk, May 10 '13 at 22:50
See this answer http://stackoverflow.com/questions/8542591/c11-reverse-range-based-for-loop — Pete Fordham, May 10 '13 at 22:51
The problem with macro'ing c++ into your own private c++-like language is that only you will understand the result.... ;) — Jeremy Friesner, May 10 '13 at 22:57
@AdityaKumar: A few characters every time you do it and several minutes every time you goof. If you do something often and you can encapsulate it in a way that keeps you from screwing up, you do it. — tmyklebu, May 10 '13 at 22:58
@Karoly Horvath I want to introduce a local variable to the loop body, without adding a { to the macro, as this will require an extra } in the main code which is odd. A one-time for loop was the only way I could think of, and I had to use the if trick to get a local bool. — Neil Kirk, May 10 '13 at 22:58
for (blah) { auto& i = *it; {UR CODE} } <--- it's this bracket that is the pest. How can I avoid the user of the macro, needing to end his loop with }} ? — Neil Kirk, May 10 '13 at 23:01
@tmyklebu, Typing a few more characters just to make sure that the code does not break is a good thing. Using macros as an encapsulation/abstraction mechanism in C++!, I think no one would appreciate that. — A. K., May 11 '13 at 16:45
@AdityaKumar: Abstraction lets you type fewer characters to make sure that the code doesn't write. At least when you do it right. Control flow macros can do this if you use them enough and engineer them right. (Ever type `for (int j = 0; j < n; i++)`? Now think of more complex scenarios, maybe like this one.) — tmyklebu, May 12 '13 at 12:17
Unfortunately my macro suffers from a massive flaw - it doesn't work with break or continue properly. — Neil Kirk, May 14 '13 at 16:01

tmyklebu · Accepted Answer · 2013-05-10T23:00:46.283

3

Not sure I like this. The if conditionals do not have corresponding else clauses, so elses within the loop body will not bind to the correct if and an if followed by too many else clauses will not cause an error.

You might try using the else clause rather than the then clause of the if instead. EDIT: Actually, since you only use the ifs to introduce variables, why not use for instead as you've done elsewhere?

Your macro also shadows whatever I've called b, which could be very annoying. Maybe use token pasting to give that a less likely name?

edited May 10 '13 at 23:00

answered May 10 '13 at 22:55

tmyklebu

13,915
3
28
57

I can't use if to introduce a real variable as it might not evaluate to true. – Neil Kirk May 10 '13 at 23:02
The for loop needs a local bool to fire exactly once. – Neil Kirk May 10 '13 at 23:05
@NeilKirk: Yes, I read your code. You can *use* a `for` loop to introduce a bunch of local `bool`s, though. – tmyklebu May 10 '13 at 23:08
Aha! I see. I also fixed it, by swapping the bool logic, and putting the fors in the else. – Neil Kirk May 10 '13 at 23:11

score 2 · Answer 2 · answered May 10 '13 at 23:11

This is a fairly subjective question so my answer will probably also be subjective.

By using this macro, you will in effect create a dialect of C++ that no one else normally fluent in C++ will be able to read comfortably at first. This isn't a problem for projects that you maintain yourself, but in any professional environment it's a fact of life that someone else will have to read or update your code in the future. You write the code one time (and this may save a small amount of coding), and people will be reading it for the lifetime of the application. For such reasons, I would suggest not using the macros.

Also note that in many cases, algorithms such as copy (to a stream iterator) and for_each (possibly in conjunction with lambdas) can very expressively do what you might try to do with a for loop, and support all directions of iteration, just by selecting which iterators you pass in to the algorithm.

dyp · Answer 3 · 2013-05-11T01:09:44.940

Suggesting a wrapper for the same/similar functionality. Community wiki as I know this is just a concept atm and needs improvement.

#include <iostream>
#include <vector>

template < typename Iter >
struct range
{
    Iter m_begin;
    Iter m_end;

    struct Iterator// : std::iterator < ... >
    {
        Iter m;

        Iter operator*() const
        { return m; }
        bool operator!= (Iterator const& p) const
        { return m != p.m; }
        Iterator& operator++()
        { ++m; return *this; }
    };

    Iterator begin()
    {
      return Iterator{m_begin};
    }
    Iterator end()
    {
      return Iterator{m_end};
    }
};

template < typename Cont >
range < typename Cont::iterator > make_range(Cont& p)
{ return {p.begin(), p.end()}; }

template < typename Cont >
range < typename Cont::reverse_iterator > make_reverse_range(Cont& p)
{ return {p.rbegin(), p.rend()}; }


int main()
{
    std::vector<int> v = {1,2,3,4,5};
    for(auto it : make_reverse_range(v))
    {
        auto var = *it;
        std::cout << var << std::endl;
    }
}

What do you think to this for loop macro?

3 Answers3