2
\$\begingroup\$

I was trying to figure out how to make a breakable foreach macro for STL containers and I came up with this method that uses templates to recognize the container type automatically. Are there any performance / code-wise improvements you can think of?

// polymorphic break-able foreach test that works with nested calls on the same type of container

#include <vector>
#include <list>
#include <iostream>
#include <unordered_map>

std::unordered_map<void *, bool> _advance_iterator__instances_done;

template <class Tc, class Ti>
bool advance_iterator(Tc &container, Ti &iterator)
{
   static typename Tc::iterator it;

   if (_advance_iterator__instances_done[&container])
   { 
      it = container.begin();
      _advance_iterator__instances_done[&container] = false;
   }   

   if (it == container.end())
   { 
      _advance_iterator__instances_done[&container] = true;   
      return false;
   }  

   iterator = *it;
   it++; 

   return true;   
}

#define foreach(value_type, iterator, container) \
_advance_iterator__instances_done[&container] = true; \
for (value_type iterator; advance_iterator(container, iterator); )

int main(int argc, char *argv[])
{
   std::vector<int> vec;
   std::list<int> lis;
   std::list<std::list<int>> matrix; 

   // populate containers 
   for (int i = 0; i < 10; i++)
   {
      static std::list<int> row; 

      vec.push_back(i);
      lis.push_back(i + 10);

      for (int j = 1; j <= 10; j++)
         row.push_back(j * i);

      matrix.push_back(row);
      row.clear();    
   }  

   // regular foreach test  
   std::cout << "vec:" << std::endl;      

   foreach(int, value, vec)
      std::cout << value << std::endl;  

   std::cout << std::endl << "half of lis:" << std::endl; 

   // breaking test 
   foreach(int, value, lis)
   { 
      std::cout << value << std::endl;

      if (value == 14)
         break;    
   }  

   // regular again 
   std::cout << std::endl << "full lis:" << std::endl;

   foreach(int, value, lis)
      std::cout << value << std::endl; 

   // nested test
   std::cout << std::endl << "matrix:" << std::endl; 

   foreach(std::list<int>, row, matrix)
   {
      foreach(int, value, row)
         std::cout << value << " "; 

      std::cout << std::endl;
   }      

   return 0;
}
\$\endgroup\$
1
\$\begingroup\$

This line gives my nightmares:

std::unordered_map<void *, bool> _advance_iterator__instances_done;

First up the name is reserved. Prefer not to use identifiers without a leading underscore (the rules are not trivial and everybody gets them wrong so just avoid them).

Secondly the size of the map is unbound. If you have a really large application (runs a long time) this is going to continuously grow.

This construct

bool advance_iterator(Tc &container, Ti &iterator)
{
   static typename Tc::iterator it;

makes the whole thing not thread safe.

Your naming convention makes it confusing to read:

#define foreach(value_type, iterator, container) \
                           ^^^^^^^^^^^
                     This is never the iterator it is the value.

Overall I think the boost FOREACH is a better choice for C++03 and C++11 now has a built in version

\$\endgroup\$
  • \$\begingroup\$ Thanks for the review, I added a cleanup to avoid the map growing in size, fixed the wrong naming for the value in the macro and wrapped everything in a namespace. How's it now? By the way, boost foreach requires you to pass the container type so I made this mainly for fun and also to shorten code :) EDIT: oh yeah boost foreach doesnt require container type, but still, I made this for fun and for learning purposes \$\endgroup\$ – Francesco Noferi Oct 2 '12 at 15:38
  • \$\begingroup\$ @FrancescoNoferi: No boost for each only requires you to pass the type of the value you want to see inside your loop. If the value_type of the container is convertible to this it will compile and convert otherwise not. std::vector<float> vec; BOOST_FOREACH(int loop, vec) { std::cout << loop << "\n";} Each value of vec is converted to the integer during assignment to loop. \$\endgroup\$ – Martin York Oct 2 '12 at 15:43
  • \$\begingroup\$ Yeah, my bad, but yeah making my own is a good learning experience and I don't need to have boost libraries just to use foreach for a project that doesn't use anything else of the boost libraries \$\endgroup\$ – Francesco Noferi Oct 2 '12 at 15:45
  • 1
    \$\begingroup\$ Any developer that does not already have boost installed is probably not doing real C++ development. Boost is now so ubiquitous that not having boost is the exception to the rule and as soon as you need it the first time you install it and always have it. It comes standard in most Linux development distributions now. \$\endgroup\$ – Martin York Oct 2 '12 at 15:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.