An "acceptable" use of break

[Larry Doolittle]

/* output the numbers entered. only print 8 numbers per line */
printf ("\nNumbers Entered:\n");
for (i = 0; i < MAX; ++i) {
if (list < 0)
break;
if (i % 8 != 0)
printf ("%-4d ", list);
else if (i == 0)
printf ("%-4d ", list);
else
printf ("%-4d\n", list);
}

This prints 9 numbers on the first line (i=0...8), and leaves a trailing
blank instead of \n at the end of the list (unless (MAX-1)%8==0). Your
one remaining use of break looks kind of silly, too. How about
/* output the numbers entered. only print 8 numbers per line */
puts ("\nNumbers Entered:");
for (i = 0; i < MAX && list < 0; ++i) {
if (i!=0) putchar ((i%8==0) ? '\n' : ' ');
printf ("%-4d", list);
}
putchar('\n');
I missed the beginning of this thread, so I'm not sure
if you really wanted to abort the display when you run
into a negative list element.

- Larry

 

[David Rubin]

printf ("Numbers Entered:");
#define NL_MASK 7
for (i = 0;i < MAX; ++i) {
if (list < 0) break;
printf ("%c%-6d", (i & NL_MASK) ? ' ' : '\n', list);
}
printf ("\n");

[snip - above code is tricky]

A trick? Not readable? Nor robust?

Tell me how your code is less of a trick and more of the
other two!

The use of the bitwise & operator to perform modular division is an
unexpected and tricky construct. Unexpected because we are dealing with
integer arithmetic, not bit manipulation. Tricky because it is difficult
(for the beginner) to understand what you are trying to do. Not robust
because it is not possible to extend your code (as written) to print a
different number of integers per line. Not readable because of the poor
use of whitespace (e.g., if() break) and the use of the ternary operator
in the printf call (again, to the beginner--I do not generally disaprove
of this style).

[snip]

/*
* print up to max non-negative numbers from list
* at npl numbers per line
*/
void
show(int list[], int max, int npl)
{
int i, j;

for(i=0, j=0; i < max && list >= 0; ++i){
printf("%d", list);
if(++j == npl){
putchar('\n');
j = 0;
}else
putchar(' ');
}
if(j > 0)
putchar('\n');
}

Well, I can't say that what you have is bad. However, by
comparison, it has an overly complex for-loop conditional,

Actually, If I'd made it (list >=0 && i < max) it might even be more
efficient, depending on the average-case input. Additionally, the
"complex" conditional expresses the entire intent of the loop: print up
to max integers, stopping at the first negative integer. This style
represents a particular, cohesive way of thinking about the solution:
given the general looping condition, how do I format the output
(expressed within the for loop)? The two concepts are not mixed as in
your example.

> I
don't generally believe that incrementing variables withingthe
if condition expression should be done when it can be avoided,

The equivalent code,

++j;
if(j == npl){...}

is a bit clumsy, IMO.

and you have four different function calls to two different
function (printf once and putchar three times).

The last putchar takes care of the special case where the last line of
output is the full npl. It's much easier to see what is happening and
why the way I wrote it; thus, more readable.

> You also use
two variables where only one is needed. And it's it ten lines
in place of four.

I could have written the same function in one line with no extra
variables. The point is readability. Source size doesn't matter much.
These are important points to understand for beginners.

I don't see one or the other as "better", but I wouldn't use
your code.

You would have a lot more work to do if the requirements changed...

/david

 

[Floyd Davidson]

printf ("Numbers Entered:");
#define NL_MASK 7
for (i = 0;i < MAX; ++i) {
if (list < 0) break;
printf ("%c%-6d", (i & NL_MASK) ? ' ' : '\n', list);
}
printf ("\n");

[snip - above code is tricky]

A trick? Not readable? Nor robust?

Tell me how your code is less of a trick and more of the
other two!

The use of the bitwise & operator to perform modular division is
an unexpected and tricky construct. Unexpected because we are
dealing with integer arithmetic, not bit manipulation.

Because *you* didn't expect it??? Are you serious?

Actually, it was there specifically because the OP wouldn't
have expected it. The point was to let him see something that
he *will* encounter, except in this particular encounter he
can play with it and learn what it does fairly easy. It was
just too good an opportunity to pass on.

Tricky
because it is difficult (for the beginner) to understand what
you are trying to do.

And that was specifically pointed out as something for the
OP to investigate as an interesting product of that particular
example. The point was to *not* do his homework without
squeezing something beneficial out of the process.

Not robust because it is not possible to
extend your code (as written) to print a different number of
integers per line.

That's just so much bs, David. I've shown *two* ways to modify
it, one with only a very slight change, to make it rather
generic.

Not readable because of the poor use of
whitespace (e.g., if() break) and the use of the ternary
operator in the printf call (again, to the beginner--I do not
generally disaprove of this style).

So you don't object to the style, only to providing tutorials
for beginners. Strange...

Actually, If I'd made it (list >=0 && i < max) it might even
be more efficient, depending on the average-case
input. Additionally, the "complex" conditional expresses the
entire intent of the loop: print up to max integers, stopping at
the first negative integer. This style represents a particular,
cohesive way of thinking about the solution: given the general
looping condition, how do I format the output (expressed within
the for loop)? The two concepts are not mixed as in your example.

The loop (and it's limits) is one concept, and what is printed
is another. Separating them, as my example did, is much clearer
at a glance. A reader need not look for how the various
statements interact. Your example is the one that blurred
various concepts.

The equivalent code,

++j;
if(j == npl){...}

is a bit clumsy, IMO.

It is *much* clearer. Especially to someone asking for a
tutorial. Hiding increments in conditionals often leads to them
being overlooked. (Both by beginners and by maintainers.)

The last putchar takes care of the special case where the last
line of output is the full npl. It's much easier to see what is
happening and why the way I wrote it; thus, more readable.

Matter of opinion. Yours is interesting. So's mine...

I don't see how anything can be easier to read when it is
twice a long and mixes concepts. YMMV, obviously.

I could have written the same function in one line with no extra
variables. The point is readability. Source size doesn't matter
much. These are important points to understand for beginners.

Source size is not the point. The point is how many lines of
code can a person "see" at one time and yet continue to relate
each to the others. (That was the purpose in using one line for
the "if (...) break;" statement. Your comments are frivolous.)

You would have a lot more work to do if the requirements changed...

With one slight modification (which true enough I perhaps should
have pointed out in my original article and didn't because the
intent was to get the OP to look at something else) it becomes
quite generic.

Whatever, in your zeal to denounce something so obvious, and so
obviously distinct from the way you would do it, you've missed
another very useful point. The OP, as with anyone asking for
that kind of assistance, benefits from seeing a variety of
approaches. Other than those who have pointed out explicit
errors in some of the attempts at answering, you seem to be the
only one who is critical of code because it is *different* than
yours. I'm sorry, but that is an invalid criterion.

 

[bd]

Hello everyone. I am taking my first course in C and in one of my
assignments
i need to print out an array that could have anywhere from 0 to 100
positive integers in it (a negative integer is used to stop input),
and i have to print 8 integers per line. The code that i have works,
and does exactly that, but i feel a little uneasy because i am using
two breaks. I would like to hear any comments about this approach, and
what i might do to make the code more elegant/readable. Thanks in
advance.

/* output the numbers entered. only print 8 numbers per line */
printf ("Numbers Entered:\n");
for (i = 0; i < MAX/8; ++i) {
for (j = pos; j < pos + 8; ++j) {
if (list[j] >= 0)
printf ("%-6d ", list[j]);
else
break;
}
if (list[j] < 0) {
printf ("\n");
break;
}
else {
printf ("\n");
pos += 8;
}
}

-Chris Potter
"Mind over matter: if you don't mind, then it doesn't matter"

Simply output a newline after every 8th number:

int i;
int list[ ... ];
for(i = 0; list >= 0; i++){
printf("%-6d ", list);
if(i % 8 == 7){
printf("\n");
}
}
if(i % 8 != 0 && i){
/* Don't output two newlines if we stop at the beginning of a line */
printf("\n");
}

 

[Mark]

Hello everyone. I am taking my first course in C and in one of my
assignments
i need to print out an array that could have anywhere from 0 to 100
positive integers in it (a negative integer is used to stop input),
and i have to print 8 integers per line. The code that i have works,
and does exactly that, but i feel a little uneasy because i am using
two breaks. I would like to hear any comments about this approach, and
what i might do to make the code more elegant/readable. Thanks in
advance.

/* output the numbers entered. only print 8 numbers per line */
printf ("Numbers Entered:\n");
for (i = 0; i < MAX/8; ++i) {
for (j = pos; j < pos + 8; ++j) {

<snip>

All students thinking about posting here, please follow Chris' lead.
A well explained problem, and a good solid crack at some code for it.


I think this following code fragment (warning, not tested!) is about
right. No real trickery, and is relatively straightforward and brief.


int list[] = { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, -1 };
#define MAX 100 /* Max elements in list */
#define NPL 8 /* Number printed per line */

void bar(void)
{
int i; /* The index into list array */
char ws; /* Which whitespace to print */

for(i = 0; i < MAX && list >= 0; i++) {
ws = (i + 1) % NPL ? ' ' : '\n';
printf("%-6d%c", list, ws);
}
}


Regards,


Mark Haigh
(e-mail address removed)

 

[David Rubin]

Whatever, in your zeal to denounce something so obvious, and so
obviously distinct from the way you would do it, you've missed
another very useful point. The OP, as with anyone asking for
that kind of assistance, benefits from seeing a variety of
approaches. Other than those who have pointed out explicit
errors in some of the attempts at answering, you seem to be the
only one who is critical of code because it is *different* than
yours. I'm sorry, but that is an invalid criterion.

I agree with you that it is important to introduce a variety of
solutions expressing different tradeoffs in style and cleverness. I'm
only trying to make the observation that being overly clever with your
code is often not appreciated by your collaborators. Granted, I tried to
make this point implicitly by pointing out why I thought your solution
was overly clever as compared to one (my own) which I feel is
more...literal. I should have stated my opinion more directly.

/david

 

[Floyd Davidson]

I agree with you that it is important to introduce a variety of
solutions expressing different tradeoffs in style and cleverness. I'm
only trying to make the observation that being overly clever with your
code is often not appreciated by your collaborators. Granted, I tried to
make this point implicitly by pointing out why I thought your solution
was overly clever as compared to one (my own) which I feel is
more...literal. I should have stated my opinion more directly.

I don't think your point was well taken to begin with. Even if
it had been, what value was there in posting. Your ego is not
on topic here.