How do I pass an Int Array into void for a foreach loop?

Question

I know its probably an odd question...but I found a workaround for a foreach loop in C. I have a whole bunch of int arrays that I would like to be able to pass into a void, which has that foreach loop in it. This is my code right now:

int example[] = {1, 2, 3, 4, 5};

void setup () {
    Serial.begin(57600);

    #define foreach(item, array) \
        for(int keep = 1, \
            count = 0,\
            size = sizeof (array) / sizeof *(array); \
            keep && count != size; \
            keep = !keep, count++) \
        for(item = (array) + count; keep; keep = !keep)
    }
}

void Words (int w[]) {
    foreach(int *v, w) {
        Serial.println(*v);
        delay(500);
    }
}

void loop () {
    Words (example);
}

However when I run it, it only returns the first number of the array, I need all 5 of them. I know theres all sorts of additional things that could be it, like the * or & symbol but I really don't understand what they do. Any ideas as to how to get that to work?

Answer 1

What you want will not work the way you think: in C++ (just like in C), arrays and pointers are almost the same, and the compiler won't be able to make a distinction between

void f(int* array)

and

void f(int array[])

In particular, inside f() the compiler won't be able to know the actual size of array: for the compiler sizeof array will always be the same as sizeof(int*), i.e. just the size of a pointer, always 2 bytes on Arduino UNO, whatever type it points to.

If you want your foreach macro to work correctly, you have to pass it the actual array count of items as an additional argument:

#define foreach(item, array, size) \
    for(int keep = 1, \
        count = 0;\
        keep && count != size; \
        keep = !keep, count++) \
    for(item = (array) + count; keep; keep = !keep)

And also pass the count of items in Words:

void Words(int w[], size_t size) {
    foreach(int *v, w, size) {
        Serial.println(*v);
        delay(500);
    }
}

In loop() you can use sizeof example, because the compiler knows what its size is, from its initialization (an array of 5 integers, i.e. 10 bytes):

void loop() {
    Words(example, sizeof example / sizeof example[0]);
}

You could even define a macro to compute the count of items in an array:

#define ARRAY_SIZE(array) (sizeof(array) / sizeof(array[0]))

It will work as long as you use it with an argument that is completely known by the compiler:

void loop() {
    Words(example, ARRAY_SIZE(example));
}

Important comments on your original code

You mentioned that you want to "save space", but defining a macro will not help in this regard, as a macro is NOT a function, hence its binary code will be repeated everywhere it is used. It can just help make your source code shorter and possibly clearer.

The way you defined your macro seems weird and overly complex to me: why use 2 for loops when you need only one? It would have been possible to define it in a simpler way, for example:

#define foreach(type, var, array, size) \
    for(type* var = array; var != array + size; var++)

It would be used as follows:

void Words(int w[], size_t size) {
    foreach(int, v, w, size) {
        Serial.println(*v);
        delay(500);
    }
}

Using C++ templates may probably help too, but this is an advanced topic. I'd recommend reading first some good book about C++ before digging into templates.

Answer 2

Adding to the reasons why it's a bad idea to define that macro:

• If you want to keep your code readable, inventing syntactic sugar and implementing it with macros is the wrong way of going about it. If the language designers did not do provide a construct, there is probably a reason. This is particularly true if you are not a complete master of the language.

• Premature optimization is always a bad idea. First, modern compilers are very good about optimizing stuff. Second, unless you have measured the performance of your code in great detail and deemed the performance unacceptable, you are wasting effort and making the code harder to maintain. Third, what you think may be an optimization may result in worse code in a future version of the compiler, or in a different hardware architecture. The list goes on.

• Never use preprocessor macros unless there is no other way of doing what you want to do (there usually is). There are countless articles detailing why you should not, but here is a short summary: macros pollute the global namespace, are hard to test and thus hard to debug, have no type checking, introduce subtle (and not-so-subtle) bugs, obscure calling semantics, and generally make the code unreadable. That we are having this discussion here is proof enough :)

• In this particular example, you are relying on the fact that the array example[] is defined (meaning: declared and storage is allocated, and in this case initialized, for it) in the same translation unit as your macro. Had this not been the case, sizeof would not have worked

• If you want to inline code, use inline functions. If you want to define constants, use const. And so on.

In your case, you are far better off declaring your arrays as vector<int> and using either iterators or the range for in combination with auto type declarations:

#include <vector>
std::vector<std::vector<int>> example = {
  {3, 1, 4, 1, 6},
  {2, 7, 1, 8, 2, 8, 1, 8},
  {5, 7, 7, 2}};

...

  for (auto v: example) {
    for (auto i: v) {
      // do something with i
    }
  }

Isn't this a lot more readable than your foreach macro?

Needless to say, you need to make sure that your tool chain supports C++11; the 1.6.7 version of the Arduino IDE running on Linux definitely does.

Obviously, there are occasional reasons to break the rules I mentioned above, but until you know exactly what you are doing, it's better not to.