A stylistic question.

[Mark Healey]

I have a function that runs through a two dimensional array several times.

01 x=map->X;
02 do
02 {
04 y=map->Y;
05 do
05 {
06 stuff();
07
08 }while (y);
09 x--'
10 }while(x);

Would it be considered good or bad form to #define lines 01-05 as
something like START_MAP_LOOP and lines 07-10 as END_MAP_LOOP?

In processing these maps I have to go over them over and over again and
this would seriously reduce the size of the source files and make them
easier to read.

 

[Richard Heathfield]

Mark Healey said:

I have a function that runs through a two dimensional array several times.

01 x=map->X;
02 do
02 {
04 y=map->Y;
05 do
05 {
06 stuff();
07
08 }while (y);
09 x--'
10 }while(x);

Would it be considered good or bad form to #define lines 01-05 as
something like START_MAP_LOOP and lines 07-10 as END_MAP_LOOP?

Why not just write a function to do this?

Presumably stuff() changes each time, but you could pass a function pointer
in to do the actual work.

In processing these maps I have to go over them over and over again and
this would seriously reduce the size of the source files and make them
easier to read.

This is kind of what functions are for.

 

[Bill Pursell]

I have a function that runs through a two dimensional array several times.

01 x=map->X;
02 do
02 {
04 y=map->Y;
05 do
05 {
06 stuff();
07
08 }while (y);
09 x--'
10 }while(x);

Would it be considered good or bad form to #define lines 01-05 as
something like START_MAP_LOOP and lines 07-10 as END_MAP_LOOP?

If I were maintaining the code, I'd probably be okay with it, in the
sense that I would curse your name for only a few days. Richard
Heathfield's suggestion to make it a function is probably better, but
does bring up an optimization point. If stuff() doesn't change with
each call, then making it a function is no big deal since it will
probably wind up being inlined, but if stuff does change and you
pass a pointer to the appropriate function, I think your run-time
is going to suffer from that overhead. I've been plagued with
a similar concern for a while now:

suppose my inner-most loop contains a single function call to
a very small function. Based on a run-time parameter, the
function changes, but for any instantiation of the program
that function remains constant. I see 2 ways to code that:

method 1)
typedef void (*my_foo)(void);
void foo1(void) { return;}
void foo2(void) { return;}
void foo3(void) { return;}

int main(int argc, char **argv)
{
my_foo select[3] = {foo1, foo2, foo3};
my_foo f;

f = select[argc-1];

do {
f();
} while (1);
}
*********************************
method 2)
int main(int argc, char **argv)
{

switch(argc) {
case 1: do foo1(); while (1);
case 2: do foo2(); while (1);
case 3: do foo3(); while (1);
}
}


In terms of style, I prefer method 1, but I'm concerned about the
extra overhead of dereferencing the global. In a case like this,
does style trump the run-time efficiency (which in this case is
probably very small)?

 

[Toni]

En/na Mark Healey ha escrit:

I have a function that runs through a two dimensional array several times.

01 x=map->X;
02 do
02 {
04 y=map->Y;
05 do
05 {
06 stuff();
07
08 }while (y);
09 x--'
10 }while(x);

Would it be considered good or bad form to #define lines 01-05 as
something like START_MAP_LOOP and lines 07-10 as END_MAP_LOOP?

Very bad style. Try to avoid define as much as possible.

In processing these maps I have to go over them over and over again and
this would seriously reduce the size of the source files and make them
easier to read.

As other have pointed out the way to do this is with a function.
Consider something like:

doMap(struct Map map)
{
int x, y;
for (x = map->X; x; --x)
for (y = map->Y; y; --y)
stuff();
}

call doMap(map) as often as you want.

Besides, I don't see you pass map, x nor y to stuff(). If you are using
global variables which are read in stuff() this is also the wrong way to
do it. Unless you have some special requirements, the right way would be:

for (x = map->X; x; --x)
for (y = map->Y; y; --y)
stuff(map, x, y);

Notice also how the "for" version is shorter than your version (3 lines
vs 10). Shorter is better as you have less places opportunity for bugs.

 

[Ian Collins]

I've been plagued with
a similar concern for a while now:

suppose my inner-most loop contains a single function call to
a very small function. Based on a run-time parameter, the
function changes, but for any instantiation of the program
that function remains constant. I see 2 ways to code that:

method 1)
typedef void (*my_foo)(void);
void foo1(void) { return;}
void foo2(void) { return;}
void foo3(void) { return;}

int main(int argc, char **argv)
{
my_foo select[3] = {foo1, foo2, foo3};
my_foo f;

f = select[argc-1];

do {
f();
} while (1);
}
*********************************
method 2)
int main(int argc, char **argv)
{

switch(argc) {
case 1: do foo1(); while (1);
case 2: do foo2(); while (1);
case 3: do foo3(); while (1);
}
}


In terms of style, I prefer method 1, but I'm concerned about the
extra overhead of dereferencing the global. In a case like this,
does style trump the run-time efficiency (which in this case is
probably very small)?

Every time, unless you absolutely have to optimise. Premature
optimisation is the root of (almost) all evil.

 

[Chris Hills]

I have a function that runs through a two dimensional array several times.

01 x=map->X;
02 do
02 {
04 y=map->Y;
05 do
05 {
06 stuff();
07
08 }while (y);
09 x--'
10 }while(x);

Would it be considered good or bad form to #define lines 01-05 as
something like START_MAP_LOOP and lines 07-10 as END_MAP_LOOP?

MISRA-C says defines should be bounded by () so that they don't have
any unexpected side effects.

If you can't put () round your define then it is probably not a safe
define and open to al sorts of invisible errors and problems.

In processing these maps I have to go over them over and over again and
this would seriously reduce the size of the source files and make them
easier to read.

The size of the source file in a free layout language is not really
relevant.

Making them easier to read is another matter.

You could use a function in this case. unless the "stuff()" is radically
different each time. i.e. completely different number and type of
statements and functions.

 

[pete]

Mark Healey wrote:

Would it be considered good or bad form to #define lines 01-05 as
something like START_MAP_LOOP and lines 07-10 as END_MAP_LOOP?

In processing these maps I have to go over
them over and over again and
this would seriously reduce the size of the source files

I would need a special reason to consider
size of source files, as being important.

and make them easier to read.

That's what macros are all about.
If a macro makes the code easier to read, then it's being used well.
If a macro makes the code harder to read, then it's being used badly.

 

[Tomás]

Mark Healey posted:

01 x=map->X;
02 do
02 {
04 y=map->Y;
05 do
05 {
06 stuff();
07
08 }while (y);
09 x--'
10 }while(x);

Would it be considered good or bad form to #define lines 01-05 as
something like START_MAP_LOOP and lines 07-10 as END_MAP_LOOP?

(You've the same name as my brother!)

Removing the typo and re-arranging the indentation:

x = map->X;

do
{
y = map->Y;

do
{
stuff();

} while (y);

x--;

} while (x);


I'm not sure if you can jump into a loop in C (you can in C++), but
nonetheless here's probably how I'd do it:

typedef int SomeType1;

typedef int SomeType2;

typedef struct Map {
SomeType1 X;
SomeType2 Y;
} Map;


inline void Mapper( Map * const p_map )
{
goto Loop1Body;

for ( SomeType1 x = p_map->X; x ; --x)
{
/* Label */ Loop1Body: /* Label */

goto Loop2Body;
for( SomeType2 y = p_map->Y; y; )
{
/* Label */ Loop2Body: /* Label */
stuff();
}
}
}

int main()
{
Map map = {};

Mapper( &map );
}


-Tomás

 

[Keith Thompson]

MISRA-C says defines should be bounded by () so that they don't have
any unexpected side effects.

If you can't put () round your define then it is probably not a safe
define and open to al sorts of invisible errors and problems.

[...]

That's a good rule if the macro is intended to be used as an
expression. Even then, it's not always necessary; for example:
#define FALSE 0
#define TRUE 1
Putting parentheses around the integer constants would be silly, IMHO.

On the other hand, another common idiom is:

#define DO_SOMETHING(args) \
do { \
/* blah blah */ \
while (0)

This is to be used in place of a statement, not an expression. (If
MISRA-C bans this kind of macro, that's fine, but it can be useful.)

 

[CBFalconer]

MISRA-C says defines should be bounded by () so that they don't
have any unexpected side effects.

If you can't put () round your define then it is probably not a
safe define and open to al sorts of invisible errors and problems.

[...]

That's a good rule if the macro is intended to be used as an
expression. Even then, it's not always necessary; for example:
#define FALSE 0
#define TRUE 1
Putting parentheses around the integer constants would be silly,
IMHO.

On the other hand, another common idiom is:

#define DO_SOMETHING(args) \
do { \
/* blah blah */ \
while (0)

This is to be used in place of a statement, not an expression. (If
MISRA-C bans this kind of macro, that's fine, but it can be useful.)

It would be more persuasive with a judicious '}' added

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>
Also see <http://www.safalra.com/special/googlegroupsreply/>

 

[Keith Thompson]

Keith Thompson wrote: [...]

On the other hand, another common idiom is:

#define DO_SOMETHING(args) \
do { \
/* blah blah */ \
while (0)

This is to be used in place of a statement, not an expression. (If
MISRA-C bans this kind of macro, that's fine, but it can be useful.)

It would be more persuasive with a judicious '}' added

Well, yeah. *blush*

 

[Herbert Rosenau]

I have a function that runs through a two dimensional array several times.

01 x=map->X;
02 do
02 {
04 y=map->Y;
05 do
05 {
06 stuff();
07
08 }while (y);
09 x--'
10 }while(x);

Would it be considered good or bad form to #define lines 01-05 as
something like START_MAP_LOOP and lines 07-10 as END_MAP_LOOP?

In processing these maps I have to go over them over and over again and
this would seriously reduce the size of the source files and make them
easier to read.

#define a macro DO_ARRAY(params...) that does all the stuff you needs
to do except stuff(). Document it well.
Replace stuff() with a macro call, name the macro so that it gets
identified as a macro easy.

#define the macro stuff() near the place it is used in MY_ARRAY - and
#undef it immediately after it is no more used.

#define the macro stuff() near the next place you needs it in
MY_ARRAY() before you calls it and #undef it when the point it is
needed is over.

Do so for all other places.

That gives you:
- one single place you have to maintain the whole MY_ARRAY
- clean macro expansion on any place you have to do different
stuff() but identical MY_ARRAY()
- you have a pice of code that is simular to selfmodifying code
but is NOT. You can replace a variable number statements with
variable meaning by having different macros (with different
parameters) on different places by simple exchanging the
content of a macro name.

Yes, you have to learn how to use the C macro processor - but that
will extend your C knowledge.

Your problem is resolved easy by planning exactly what are the
differences between each occurence of the general logic and defining
the places by name.

#define and #undef are created exactly for such cases where you have
multiple times the same code with little differences, making a single
fuction impossible or at least opaque.


--
Tschau/Bye
Herbert

Visit http://www.ecomstation.de the home of german eComStation
eComStation 1.2 Deutsch ist da!

 

[Ian Collins]

#define a macro DO_ARRAY(params...) that does all the stuff you needs
to do except stuff(). Document it well.
Replace stuff() with a macro call, name the macro so that it gets
identified as a macro easy.

Why use macros when a function would do?

 

[James Dow Allen]

Mark Healey said:
Why not just write a function to do this? ...
Presumably stuff() changes each time, but you could pass a function pointer
in to do the actual work.

I use this approach (though the invocation is for a line,
not a single pixel) in my image-processing library, but
only because two conditions are met: the once-per-line
processing is always the same, and it is rather involved
(buffer pointer rotation, incremental file IO, line padding,
and zigzag-adjust for hexagon-grid images).

Though I'm sure I'm in a minority (as usual!) I *avoid* the
approach (which is called what, BTW? "general cover routine"?)
when the only purpose is to avoid defining a macro, or replicating
a small amount of code. What's wrong with macros, anyway?

Even without using macros, the readability burden of replicated
code is lessened by stylistic deviations to conserve vertical
space. As a trivial example, the reader will quickly get used to
for (y = 0; y < Hei; y++)
for (x = 0; x < Wid; x++)
foobar(x, y);

when used *consistently* as a substitute for the "correct",
but space-wasting:
for (y = 0; y < Hei; y++)
{
for (x = 0; x < Wid; x++)
{
foobar(x, y);
}
}

When written "normally", an operation is invoked, controlled and
executed at a single place in the source code. Using a cover
routine, the invocation, control and execution occur in *three*
separate functions, and the reader will have to scroll or
open other files to refresh his understanding. Subdividing
functions is not an unmixed blessing -- surely good source locality
is also a virtue.

Another reason to avoid "general cover routines" is that sometimes
you need to vary the control. (I suppose "orthodox" coverers
end up with lots of seldom-needed parameters to the cover routine.)
As a case in point, recall a discussion of operating on every
element from the on-going discussion of hash table management design.
I code a straightforward loop, while my "competitor" insists that
a general cover be called. In the previous thread I indicated a
case where the general cover *cannot* work.


James Dow Allen


``Let me assert my firm belief that the only thing we have to fear
is fear itself -- nameless, unreasoning, unjustified terror which
paralyzes needed efforts to convert retreat into advance.''
-- Franklin D. Roosevelt

``When I was coming up, it was a dangerous world, and you
knew exactly who they were. It was us versus them, and
it was clear who them was. Today, we are not so sure who
the they are, but we know they're there.''
-- George W. Bush

 

[CBFalconer]

.... snip ...

Even without using macros, the readability burden of replicated
code is lessened by stylistic deviations to conserve vertical
space. As a trivial example, the reader will quickly get used to
for (y = 0; y < Hei; y++)
for (x = 0; x < Wid; x++)
foobar(x, y);

when used *consistently* as a substitute for the "correct",
but space-wasting:
for (y = 0; y < Hei; y++)
{
for (x = 0; x < Wid; x++)
{
foobar(x, y);
}
}

Why those distortions, when a normal and readable version is:

for (y = 0; y < Hei; y++)
for (x = 0; x < Wid; x++) foobar(x, y);
...

or even, for some devotees to useless braces:

for (y = 0; y < Hei; y++)
for (x = 0; x < Wid; x++) {
foobar(x, y);
}

--
"If you want to post a followup via groups.google.com, don't use
the broken "Reply" link at the bottom of the article. Click on
"show options" at the top of the article, then click on the
"Reply" at the bottom of the article headers." - Keith Thompson
More details at: <http://cfaj.freeshell.org/google/>
Also see <http://www.safalra.com/special/googlegroupsreply/>

 

[Herbert Rosenau]

Why use macros when a function would do?

When you have 30lines of code unchanged on 100% of usage followed by a
number of lines you knows not what that may be, followed by 31 lines
of code followed by a number of lines you knows nothing about .....

having a lot of differet variables using before, in middle of and
after the unkown points of code, whereas the unknown code uses
different variables you would never find a way to write a function
that will place in exactly the code you needs while calling it - but
having a macro the preprocessor will do that magically for you.

You'll get the variant code in place instead to have if's or case
statements.

#define M_X(a, b, c) { \
...30 lines.... \
a; \
...20 lines.... \
b; \
---44 lines---- \
c; \
---5--lines---- \
}

#define A blah = blubber(x, y, "hi, hi")
#define B(p) z = b; d = y; f(p)
#define C(t) x = a;

M_X(A, B(friend), C("hello");
#undef A
#define A printf("blah, blubber\");

M_X(A, B(sister), C("will die");

#undef B
#undef C
.........

Having all lines in M_X to handle common data makes it impossible to
write 5 functions having all the data in place without having them at
kleast at file level defined and therfor the problem that any other
function may use them without control.

Having one macro that receives the relevant differences on the
relevant places gives you any control you needs to make maintenance
easy, extend the functionality given with the known points of change.

Yes, that is nothing you would do in any application you has to write
in your live, but when you sits on a problem like the OP does it is
the solution.

--
Tschau/Bye
Herbert

Visit http://www.ecomstation.de the home of german eComStation
eComStation 1.2 Deutsch ist da!

 

[Ian Collins]

When you have 30lines of code unchanged on 100% of usage followed by a
number of lines you knows not what that may be, followed by 31 lines
of code followed by a number of lines you knows nothing about .....

having a lot of differet variables using before, in middle of and
after the unkown points of code, whereas the unknown code uses
different variables you would never find a way to write a function
that will place in exactly the code you needs while calling it - but
having a macro the preprocessor will do that magically for you.

You refactor the code to something more managable I've seen too many
uses of macros to cover over smelly code.

 

[James Dow Allen]

James Dow Allen wrote:
Why those distortions, when a normal and readable version is:

for (y = 0; y < Hei; y++)
for (x = 0; x < Wid; x++) foobar(x, y);

(BTW, I'm glad no one suggested the formula with a single "for"! ):
for (x = y = 0; x < Wid || (x = 0, ++y) < Hei; x++)
foobar(x, y);

In such a trivial case we can agree to quibble, or, better yet,
disdain. But suppose one is doing a 3-D convolution:

for (x = 0; x < Wid; x++)
for (y = 0; y < Hei; y++)
for (z = 0; z < Dep; z++) {
valu = 0;
for (xx = 0; xx < Filt->wid; xx++)
for (yy = 0; yy < Filt->hei; yy++)
for (zz = 0; zz < Filt->dep; zz++) {
valu += Isig[x+xx+Xoff][y+yy+Yoff][z+zz+Zoff]
* Filt->kern[xx][yy][zz];
}
Osig[x][y][z] = valu;
}

Anyone who will claim that this loop becomes "more readable" when four
more levels of indentation are added has probably been over-indulging
in ... (well, in another newsgroup, someone has taken to rhyming James
with Flames and blames so let's avoid further charges of hyperbole).

Anyway, the location of horizontal white space was by far the least
substantive of any issue I raised in my message: should we assume you
agree with the rest?

James Dow Allen

 

[Andrew Poelstra]

James Dow Allen wrote:
Why those distortions, when a normal and readable version is:

for (y = 0; y < Hei; y++)
for (x = 0; x < Wid; x++) foobar(x, y);

(BTW, I'm glad no one suggested the formula with a single "for"! ):
for (x = y = 0; x < Wid || (x = 0, ++y) < Hei; x++)
foobar(x, y);

In such a trivial case we can agree to quibble, or, better yet,
disdain. But suppose one is doing a 3-D convolution:

for (x = 0; x < Wid; x++)
for (y = 0; y < Hei; y++)
for (z = 0; z < Dep; z++) {
valu = 0;
for (xx = 0; xx < Filt->wid; xx++)
for (yy = 0; yy < Filt->hei; yy++)
for (zz = 0; zz < Filt->dep; zz++) {
valu += Isig[x+xx+Xoff][y+yy+Yoff][z+zz+Zoff]
* Filt->kern[xx][yy][zz];
}
Osig[x][y][z] = valu;
}

Anyone who will claim that this loop becomes "more readable" when four
more levels of indentation are added has probably been over-indulging
in ... (well, in another newsgroup, someone has taken to rhyming James
with Flames and blames so let's avoid further charges of hyperbole).

Anyway, the location of horizontal white space was by far the least
substantive of any issue I raised in my message: should we assume you
agree with the rest?

Well, you could indent by less than 8 spaces:

for (x = 0; x < Wid; x++)
for (y = 0; y < Hei; y++)
for (z = 0; z < Dep; z++)
{
valu = 0;
for (xx = 0; xx < Filt->wid; xx++)
for (yy = 0; yy < Filt->hei; yy++)
for (zz = 0; zz < Filt->dep; zz++)
{
valu += Isig[x+xx+Xoff][y+yy+Yoff][z+zz+Zoff]
* Filt->kern[xx][yy][zz];
}

Osig[x][y][z] = valu;
}

Now my deepest indentation is less than yours, with full indenting.
And it was /far/ easier to figure out what loop what is in.

 

[Bill Pursell]

James Dow Allen wrote:

In such a trivial case we can agree to quibble, or, better yet,
disdain. But suppose one is doing a 3-D convolution:

for (x = 0; x < Wid; x++)
for (y = 0; y < Hei; y++)
for (z = 0; z < Dep; z++) {
valu = 0;
for (xx = 0; xx < Filt->wid; xx++)
for (yy = 0; yy < Filt->hei; yy++)
for (zz = 0; zz < Filt->dep; zz++) {
valu += Isig[x+xx+Xoff][y+yy+Yoff][z+zz+Zoff]
* Filt->kern[xx][yy][zz];
}
Osig[x][y][z] = valu;
}

Anyone who will claim that this loop becomes "more readable" when four
more levels of indentation are added has probably been over-indulging
in ... (well, in another newsgroup, someone has taken to rhyming James
with Flames and blames so let's avoid further charges of hyperbole).

Well, you could indent by less than 8 spaces:

for (x = 0; x < Wid; x++)
for (y = 0; y < Hei; y++)
for (z = 0; z < Dep; z++)
{
valu = 0;
for (xx = 0; xx < Filt->wid; xx++)
for (yy = 0; yy < Filt->hei; yy++)
for (zz = 0; zz < Filt->dep; zz++)
{
valu += Isig[x+xx+Xoff][y+yy+Yoff][z+zz+Zoff]
* Filt->kern[xx][yy][zz];
}

Osig[x][y][z] = valu;
}

Now my deepest indentation is less than yours, with full indenting.
And it was /far/ easier to figure out what loop what is in.

I disagree that it's easier. I actually very much like the
non-indented
nested loop style.

This is going to change the subject a bit, but still talking about
style, so I'll leave it in this thread. I've never found an
indentation
scheme for line continuations that I like. What do you think
about this:

#define ____ /* to denote a continued line*/

if (really_long_name -> a &&
____ (b == 8) && (c & 0x08) ||
____ (another_boolean_condition) &&
____ (yet_another() == 2) ) {
do _stuff()
}

I want something at the start of the line with the
same level of indentation as the "if". "..." would
be better than underscores, but "..." would clash
with variadic function decldarations, and "...." isn't
a valid name for a macro.

At the moment I think using ____ is a pretty poor
idea, but I'd really like to hear suggestions on
better ways to do continuations. I've thought about
column-aligning '\' at the end of the lines way out
to the right, but it's not as much of a visual cue
as I'd like. half or double indenting the continued
line just doesn't work as well as it ought.