4

Some background to the issue

if I have a struct like

typedef struct {
    idx_type type;
    union {
        char *str;
        int num;
    } val
} cust_idx;

and I have loops like this 

for (i = 0; i < some_get(x); i++) {
    some_fun(z, NULL, i);
}

that I want to refactor to use the struct like some_fun(z, idx) where idx is one of my cust_idx structs, would it be best to keep i as the loop counter and update idx or change the for header to use idx.val.num instead of i?

For the purposes of this, assume idx_type is an enum for string and number types, and all other fields will have macros, but I'm only going to use the IDX_NUM macro here as I'm not worried about anything to do with idx.type.

To sum up my concerns:

  1. Will it be readable? I don't want to leave behind a mess that someone will read and just shake their head...
  2. Is it advised against?
  3. Which of these is the best solution?

Struct field as loop counter

#define IDX_NUM(x) (x.val.num)
...
cust_idx j;
j.type = TYPE_num;
for (IDX_NUM(j) = 0; IDX_NUM(j) < some_get(x); IDX_NUM(j)++) {
    some_fun(z, j);
}

This does the same as the original, but the using struct field/macro extends and complicates the for loop header in my opinion but it's still fairly understandable.

Modify struct with original counter

cust_idx j;
j.type = TYPE_num;
for (i = 0; i < some_get(x); i++) {
    IDX_NUM(j) = i;
    some_fun(z, j);
}

This results in the least changes from old code logically, but will end in by far the largest amount of code due to the add assignment lines.

Pointer to struct field

cust_idx j;
int *i = &(j.val.num);
j.type = TYPE_num;
for ((*i) = 0; (*i) < some_get(x); (*i)++) {
    some_fun(z, j);
}

I'm not sure how good this would be in the long run, or if it's advised against.

Grzegorz
  • 107
  • 8
  • I would refactor the code, so that `some_fun` takes only element pointer not the whole array. Or it takes the whole array with element pointer or the whole array with index number. But storing the index number inside the object is strange and I think against OOP - you are storing iterator inside an object. You can go with somethink along `readdir_r` but there are no functions that take both `readdir_r` arguments and iterator - each object is aware and is a separate entity. – KamilCuk Nov 29 '18 at 12:02

1 Answers1

3

As to readability, I would always prefer separate loop counters.

EDIT: The following in italic is not right in this specific case as C structs by default are passed as value copies over the stack, so passing j to some_fun() in the loop is ok. But I'll leave the caveat here, as it applies to many similar situations, where the struct or array is passed by a pointer value. (aka 'passed by reference').

That is especially true in code like you posted, where you call a function with the structure as an argument inside the loop. If I don't know what some_fun() does, I can only hope that the struct's member that I use as a loop counter is not modified. And hope is not a strategy.

So, unless there are very hard reasons for doing otherwise, I'd always place readability first. Remember: If you write code that is at the limits of your own syntactic and semantic capabilities, you will have very little fun debugging such code, as debugging is an order of magnitude harder than writing (buggy) code. ;)

Addition: You could look at the disassemblies of all variants. The compiler might do a lot of optimizations here, especially if it can 'see' some_fun().

GermanNerd
  • 643
  • 5
  • 12
  • Your point about `some_fun()` is fair, though it doesn't pass the idx `j`, but instead a different variable `x`, so there is no risk of the struct being modified. I feel like separate loop counters is definitely the norm, so I understand where you're coming from. – Grzegorz Nov 29 '18 at 12:12
  • Looking at your last example, some_fun() does get passed j, not x. But you are still right as structures in C get copied on passing...Still, I'm very wary with such constructs. – GermanNerd Nov 29 '18 at 12:21
  • Ah I misread that as `some_get()` sorry! I think that's a fair warning though.. it only takes `j` to be passed by reference for there to be a possibility of interference with the loop counter. Looks like the separate counter is indeed the winner - thank you for your edit too :) – Grzegorz Nov 29 '18 at 14:37
  • You are welcome. Another note to 'readability' questions: C coders tend to enjoy what they call 'terse' code. And indeed it is fun to write things that are super-compact, possibly one-liners.... But I doubt if there has ever been a coder who had to go back to code they wrote themselves months, if not years ago, and be utterly stumped. Readability of code reduces commenting/documentation needs, and improves maintainability. So I always go for readability, unless I need to do some serious optimizations. But each to his/her own tastes... – GermanNerd Nov 29 '18 at 14:43