An "acceptable" use of break

[Chris Potter]

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"

[Alex]

Chris Potter <ki**********@yahoo.com> wrote:

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;
}
}

How about:

for(i = 0; (i < MAX) && (list[i] >= 0); i++)
{
printf("\t%d", list[i]);
if((i + 1) % 8 == 0)
putchar('\n');
}

if((i + 1) % 8 != 0)
putchar('\n');
Alex

[Sidney Cadot]

Chris Potter wrote:

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.

Just to get a clear definition of what your code must do:

* according to your code, a zero can be printed and does not terminate
the list (i.e., the 'positive' in your problem definition should read
'non-negative'). Is this correct?
* is it already guaranteed that the list will contain a negative
number at the end? In that case, your 'list' array should be
able to hold 101 values, and the compare with MAX is unnecessary.
* should a newline be printed when the list contains zero non-negative
(or positive) elements?
* should a newline be printed at the end when the number of non-negative
(or positive) elements in your list is not divisible by eight?
* how many newlines must be printed at the end when the number of
non-negative (or positive) elements in your list is non-zero, but
divisible by eight?

A big part of the learning experience in exercises like these is
thinking about these boundary cases, and making sure these are properly
handled. If you provide answers, I'll try to come up with the smallest
C program that does this, probably using one 'for' loop and zero
'break's. That way, it's a nice exercise for me as well :-)

Best regards,

Sidney

[Andy]

Chris Potter wrote:

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"

i'm kind of new to C also, but how about this code:
is it acceptable, efficient... and so on. thanx.

#include <stdio.h>

#define MAX 101
main()
{
int list[MAX];
int counter = 0;
printf("Enter numbers from 0-100 (-1 or > 100) to exit: ");
do {
scanf("%d", &list[counter]);
}while((list[counter] >= 0)&&(list[counter]<=MAX-1)&&(counter++ < MAX));

int j;
for (j = 0; j < MAX; j++) {
if (j % 8 == 0)
putchar('\n');
if (j > 0 && list[j] <= 0 || list[j] > MAX -1)
break;
printf("%-6d", list[j]);
}
putchar('\n');
}

[Floyd Davidson]

ki**********@yahoo.com (Chris Potter) wrote:

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;
}
}

There's nothing really wrong with that. It just, ah... lacks a
bit in the area of elegance. (But you knew that.)

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

However, that is not without its quirks. If NL_MASK is defined
as 3, you'll get 4 columns; and if it is defined as 15 you'll
get 16 columns. But other numbers in between do not work. I'll
leave that as an exercise for you to figure out. (Think of
NL_MASK as a bit mask, and pay attention to the least
significant bit, and it will become obvious.)

To make it work with any number of columns, I'd suggest using
another variable, incremented when i is incremented, but reset
to zero when a newline is output. It ends up with two variables
rather than the three you were using, but there will only be one
loop instead of two.

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

--
Floyd L. Davidson <http://web.newsguy.com/floyd_davidson>
Ukpeagvik (Barrow, Alaska) fl***@barrow.com

[Chris]

Hello Sidney. Sorry about the ambiguity.

* according to your code, a zero can be printed and does not terminate
the list (i.e., the 'positive' in your problem definition should read
'non-negative'). Is this correct?
The Code should permit a zero to be printed, so you are correct that i
should have stated "non-negative".
* is it already guaranteed that the list will contain a negative
number at the end? In that case, your 'list' array should be
able to hold 101 values, and the compare with MAX is unnecessary.
There is not a negative value at the end of list[] by default.The user
enters that whenever they are ready to stop input. (i realize that if the
user only enters a few numbers that the rest of the array would be
"wasted" memory, but we haven't gotten to any memory allocation techniques
yet.)
* should a newline be printed when the list contains zero non-negative
(or positive) elements?
The exercise itself is not that precisely defined. What i think should
happen is that the output will display 8 of the "non-negative" integers
enterd, and then a newline. If there are none to be output then i think
one newline would be correct.
* should a newline be printed at the end when the number of non-negative
(or positive) elements in your list is not divisible by eight?
Again, the exercise was not defined that precisely, but i think that would
be the right way to do it. That is the way the code i have now outputs it.
* how many newlines must be printed at the end when the number of
non-negative (or positive) elements in your list is non-zero, but
divisible by eight?

I'm not sure that i understand your question, but if i do understand it,
there should be one newline per 8 integers printed.

Here is the exact wording of the exercise i am working with:

"Write a program that reads an array of at most 100 positive integers.
The user will indicate the end of input by entering a negative number. The
program should then output the list of numbers, eight numbers per line."

Thanks for the tips/info/help.

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

[David Rubin]

Chris wrote:

* should a newline be printed when the list contains zero non-negative
(or positive) elements?
Typically, no. This is like asking whether 'echo' should print a newline
when given an empty argument list.

[snip]* should a newline be printed at the end when the number of non-negative
(or positive) elements in your list is not divisible by eight?
Typically, yes, you should always print a newline at the end of each
line of output.

[snip]* how many newlines must be printed at the end when the number of
non-negative (or positive) elements in your list is non-zero, but
divisible by eight?

Typically, just one, at the end of the last full line of output.

/david

--
"As a scientist, Throckmorton knew that if he were ever to break wind in
the echo chamber, he would never hear the end of it."

[David Rubin]

Floyd Davidson wrote:

[snip - a solution]

There's nothing really wrong with that. It just, ah... lacks a
bit in the area of elegance. (But you knew that.)

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

Don't confuse elegence with l33t-ness. The 'problem' you are trying to
avoid is modular division. While % is usually regarded as a 'slow'
operation, there is really no sense in sidestepping it when performance
is not an issue. The code above relies on a trick to perform modular
division by 8. However, the code is neither readable, nor robust.

How about simply

/*
* 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[i] >= 0; ++i){
printf("%d", list[i]);
if(++j == npl){
putchar('\n');
j = 0;
}else
putchar(' ');
}
if(j > 0)
putchar('\n');
}

/david

--
"As a scientist, Throckmorton knew that if he were ever to break wind in
the echo chamber, he would never hear the end of it."

[Sheldon Simms]

On Wed, 05 Nov 2003 17:13:52 -0800, Chris Potter wrote:

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;
}
}

This code has a subtle bug.
Hint: try changing MAX to 12 and entering 10 numbers

As for your question, you are right. The two breaks in the code is a bit
clumsy. One break leaves the inner loop, where you then immediately test
for the break condition again and then break again. A cleaner way to do
this is:

/* Note: this code also has the bug, I'm going to let you find
it yourself with my hint. */

for (i = 0; i < MAX/8; ++i) {
for (j = pos; j < pos + 8; ++j) {
if (list[j] >= 0)
printf ("%-6d ", list[j]);
else
goto end_of_loop;
}

printf ("\n");
pos += 8;
}

end_of_loop:
printf("\n");

Now this approach might give your instructor a heart attack, and it
really is not very nice, but it *is* better than your double break.
An approach better than either of these would be to use a single loop
and just test for when it's time to print a newline:

i = 0;
while (i < MAX && list[i] >= 0)
{
printf("%-6d", list[i]);

i += 1;
if (i % 8 == 0)
putchar('\n');
else
putchar(' ');
}

if (i > 0 && i % 8 != 0)
putchar('\n');

-Sheldon

[Sheldon Simms]

On Wed, 05 Nov 2003 21:15:12 -0500, Andy wrote:

i'm kind of new to C also, but how about this code:
is it acceptable, efficient... and so on. thanx.

#include <stdio.h>

#define MAX 101
main()
Write it this way:
int main (void)
{
int list[MAX];
int counter = 0;
printf("Enter numbers from 0-100 (-1 or > 100) to exit: ");
do {
scanf("%d", &list[counter]);
}while((list[counter] >= 0)&&(list[counter]<=MAX-1)&&(counter++ < MAX));

int j; ^^^^^
Be careful. Declarations mixed in with code are allowed in the current C
standard, but some compilers do not allow it yet. Why not just put it up
at the top before the declaration of list?
for (j = 0; j < MAX; j++) {
You know that there are exactly 'counter' numbers. Why are you bothering
to loop through the entire array?
if (j % 8 == 0)
putchar('\n');
if (j > 0 && list[j] <= 0 || list[j] > MAX -1)
I would suggest some extra parentheses in this if-condition. I suspect
this condition tests something slightly different that you think it does.

I'm not quite sure why you are testing for j > 0 anyway. If the first
number I type is -1, then your code will print the -1. Is that what you
want?

Lastly, this condition will prevent integers with the value 0 from
being printed. You probably don't want that.
break;
printf("%-6d", list[j]);
}
putchar('\n');
Add this:
return 0;
}

After all that nitpicking, I should say that your code isn't awful. It
works almost (not completely) correctly, and I like that you used a single
loop with the % operator.

-Sheldon

[Floyd Davidson]

David Rubin <fu******@warpmail.net> wrote:

Floyd Davidson wrote:

[snip - a solution]

There's nothing really wrong with that. It just, ah... lacks a
bit in the area of elegance. (But you knew that.)
printf ("Numbers Entered:");
#define NL_MASK 7
for (i = 0;i < MAX; ++i) {
if (list[i] < 0) break;
printf ("%c%-6d", (i & NL_MASK) ? ' ' : '\n', list[i]);
}
printf ("\n");
Don't confuse elegence with l33t-ness. The 'problem' you are
trying to avoid is modular division. While % is usually regarded
as a 'slow' operation, there is really no sense in sidestepping
it when performance is not an issue. The code above relies on a
trick to perform modular division by 8. However, the code is
neither readable, nor robust.

A trick? Not readable? Nor robust?

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

I'd have been more impressed if you had suggested that rather
than the two variables I recommented at the end of my post, a
better method would be to substitute this loop:

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

Now NL_MASK can have any value, and we still have one loop and
one variable. Note that the %c format specifier could also
be made to be a string, %s; and a other odd things, such as
line numbers or page breaks are also easy to add.
How about simply

/*
* 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[i] >= 0; ++i){
printf("%d", list[i]);
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, I
don't generally believe that incrementing variables withingthe
if condition expression should be done when it can be avoided,
and you have four different function calls to two different
function (printf once and putchar three times). You also use
two variables where only one is needed. And it's it ten lines
in place of four.

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

--
Floyd L. Davidson <http://web.newsguy.com/floyd_davidson>
Ukpeagvik (Barrow, Alaska) fl***@barrow.com

[Andy]

Sheldon Simms wrote:

On Wed, 05 Nov 2003 21:15:12 -0500, Andy wrote:

i'm kind of new to C also, but how about this code:
is it acceptable, efficient... and so on. thanx.

#include <stdio.h>

#define MAX 101
main()

Write it this way:
int main (void)

{
int list[MAX];
int counter = 0;
printf("Enter numbers from 0-100 (-1 or > 100) to exit: ");
do {
scanf("%d", &list[counter]);
}while((list[counter] >= 0)&&(list[counter]<=MAX-1)&&(counter++ < MAX));

int j;

^^^^^
Be careful. Declarations mixed in with code are allowed in the current C
standard, but some compilers do not allow it yet. Why not just put it up
at the top before the declaration of list?

for (j = 0; j < MAX; j++) {

You know that there are exactly 'counter' numbers. Why are you bothering
to loop through the entire array?

if (j % 8 == 0)
putchar('\n');
if (j > 0 && list[j] <= 0 || list[j] > MAX -1)

I would suggest some extra parentheses in this if-condition. I suspect
this condition tests something slightly different that you think it does.

I'm not quite sure why you are testing for j > 0 anyway. If the first
number I type is -1, then your code will print the -1. Is that what you
want?

Lastly, this condition will prevent integers with the value 0 from
being printed. You probably don't want that.

break;
printf("%-6d", list[j]);
}
putchar('\n');

Add this:
return 0;

}

After all that nitpicking, I should say that your code isn't awful. It
works almost (not completely) correctly, and I like that you used a single
loop with the % operator.

-Sheldon

Thanx for the reply, And yest you have a point, why bother going through the whole array
when I have counter. (thanks, stupid newbee mistake)

if(j > 0 && list[j] <= 0 || list[j] > MAX - 1)
probably should have done it like this
if((list[j] < 0) || (list[j] > MAX - 1))
what do you think?

the declaring variables in with code...
whats usually good programming practice?
thanks again.

[Andy]

Sheldon Simms wrote:

On Wed, 05 Nov 2003 21:15:12 -0500, Andy wrote:

i'm kind of new to C also, but how about this code:
is it acceptable, efficient... and so on. thanx.

#include <stdio.h>

#define MAX 101
main()

Write it this way:
int main (void)

{
int list[MAX];
int counter = 0;
printf("Enter numbers from 0-100 (-1 or > 100) to exit: ");
do {
scanf("%d", &list[counter]);
}while((list[counter] >= 0)&&(list[counter]<=MAX-1)&&(counter++ < MAX));

int j;

^^^^^
Be careful. Declarations mixed in with code are allowed in the current C
standard, but some compilers do not allow it yet. Why not just put it up
at the top before the declaration of list?

for (j = 0; j < MAX; j++) {

You know that there are exactly 'counter' numbers. Why are you bothering
to loop through the entire array?

if (j % 8 == 0)
putchar('\n');
if (j > 0 && list[j] <= 0 || list[j] > MAX -1)

I would suggest some extra parentheses in this if-condition. I suspect
this condition tests something slightly different that you think it does.

I'm not quite sure why you are testing for j > 0 anyway. If the first
number I type is -1, then your code will print the -1. Is that what you
want?

Lastly, this condition will prevent integers with the value 0 from
being printed. You probably don't want that.

break;
printf("%-6d", list[j]);
}
putchar('\n');

Add this:
return 0;

}

After all that nitpicking, I should say that your code isn't awful. It
works almost (not completely) correctly, and I like that you used a single
loop with the % operator.

-Sheldon

Thanx for the reply, And yest you have a point, why bother going through the whole array
when I have counter. (thanks, stupid newbee mistake)

if(j > 0 && list[j] <= 0 || list[j] > MAX - 1)
probably should have done it like this
if((list[j] < 0) || (list[j] > MAX - 1))
what do you think?

the declaring variables in with code...
whats usually good programming practice?

and the main() with out int main(void)..
I did it, thats the way I saw it in The C Programming Language(main()).
but I have seen many ppl suggest int main(void), just didnt think about
but thank you once again.

[Sheldon Simms]

On Thu, 06 Nov 2003 00:07:08 -0500, Andy wrote:

if(j > 0 && list[j] <= 0 || list[j] > MAX - 1)
probably should have done it like this
if((list[j] < 0) || (list[j] > MAX - 1))
what do you think?
Looks fine to me. By the way, I'm not trying to say that you have to put
parentheses around every operator, it's just that I suspect that what you
wanted to test was not what you were actually testing in the original
code. Getting rid of the test for j > 0 made the whole condition simpler
and easier to understand anyway.
the declaring variables in with code...
whats usually good programming practice?

It's probably best to avoid it for now in C code. It's a new thing in C
(it's been aroung in C++ for a long time though) and you don't want to
write a bunch of C code that mixes declarations and code only to have to
rewrite everything when you want to compile on a compiler that gives you
syntax errors.

That said, for your throwaway homework assignments, it's not that big a
deal one way or the other, IMHO.

-Sheldon

p.s. Even though "throwaway" code usually doesn't get thrown away...

[Andy]

Sheldon Simms wrote:

On Thu, 06 Nov 2003 00:07:08 -0500, Andy wrote:

if(j > 0 && list[j] <= 0 || list[j] > MAX - 1)
probably should have done it like this
if((list[j] < 0) || (list[j] > MAX - 1))
what do you think?

Looks fine to me. By the way, I'm not trying to say that you have to put
parentheses around every operator, it's just that I suspect that what you
wanted to test was not what you were actually testing in the original
code. Getting rid of the test for j > 0 made the whole condition simpler
and easier to understand anyway.

the declaring variables in with code...
whats usually good programming practice?

It's probably best to avoid it for now in C code. It's a new thing in C
(it's been aroung in C++ for a long time though) and you don't want to
write a bunch of C code that mixes declarations and code only to have to
rewrite everything when you want to compile on a compiler that gives you
syntax errors.

That said, for your throwaway homework assignments, it's not that big a
deal one way or the other, IMHO.

-Sheldon

p.s. Even though "throwaway" code usually doesn't get thrown away...

well you are right, putting parentheses makes the code look clearer
and i'm very anal when it comes to doing things the right way.
I'll keep in mind not putting declarations mixed with code in C.

thanks
cheers

[Christian Bau]

In article <6d*************************@posting.google.com> ,
ki**********@yahoo.com (Chris Potter) wrote:

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;
}
}

One bug: pos must be initialised to 0 somewhere.

The first break is removed quite easily:

for (j = pos; j < pos + 8 && list [j] >= 0; ++j) {
printf ("%-6d ", list[j]);
}

The second one is removed quite easily as well:

for (i = 0; i < MAX/8 && list[pos] >= 0; ++i) {
...
}

The variable "i" is just artificial and not needed. I would write

for (pos = 0; pos < MAX && list [pos] >= 0; pos += 8) {
for (j = pos; j < pos + 8 && j < MAX && list [j] >= 0; ++j) {
printf ("%-6d ", list[j]);
}
printf ("\n");
}

This handles the case that MAX is not a multiple of eight as well.
However, this strategy isn't very general. You might have a more
complicated situation, where some items are skipped, and there might be
other things that make it more complicated. What you can do is something
like this:

int items_in_line = 0;
for (j = 0; j < MAX; ++j) {
if ("item j should not be printed") continue;
if ("end of list of items") break;

if (items_in_line == 8) {
printf ("\n");
items_in_line = 0;
}
printf ("%-6d ", list[j]);
items_in_line += 1;
}
if (items_in_line > 0)
printf ("\n");

This lets you easily add code to do things at the start of each line;
and it is easily extended if you have to format pages or if you want to
add a blank line after every tenth line.

[Christian Bau]

In article <2u*********************@twister.nyc.rr.com>,
David Rubin <fu******@warpmail.net> wrote:

Floyd Davidson wrote:

[snip - a solution]

There's nothing really wrong with that. It just, ah... lacks a
bit in the area of elegance. (But you knew that.)

printf ("Numbers Entered:");
#define NL_MASK 7
for (i = 0;i < MAX; ++i) {
if (list[i] < 0) break;
printf ("%c%-6d", (i & NL_MASK) ? ' ' : '\n', list[i]);
}
printf ("\n");
Don't confuse elegence with l33t-ness. The 'problem' you are trying to
avoid is modular division. While % is usually regarded as a 'slow'
operation,

usually by beginners
there is really no sense in sidestepping it when performance
is not an issue. The code above relies on a trick to perform modular
division by 8. However, the code is neither readable, nor robust.

A very good criterion for the quality of code is this: If I make a tiny
change in the requirements, what kind of changes do you have to make in
your code?

One simple change: Instead of writing a function that prints eight items
per line, write two functions, one that writes eight and one that writes
sixteen items per line.

Trouble: He forgot to #undef NL_MASK, so duplication the function and
changing "7" to "15" doesn't work.

Another simple change: Instead of writing eight items per line, write
nine items per line. Change 7 to 8 and start debugging...
And I must say, if a programmer tries to save a nanosecond by replacing
x % 8 with (x & 0x07) _and then calls printf_ in the next line, then I
doubt that this programmer will ever write efficient code.

[Chris]

On Wed, 05 Nov 2003 23:25:09 -0500, Sheldon Simms wrote:

This code has a subtle bug.
Hint: try changing MAX to 12 and entering 10 numbers
Ah-ha! I didn't even notice that bug. Thank you for pointing it out. I was
using MAX values of multiples of 8 for testing purposes and was planning
to just make MAX 100 when i was done, which would result in the bug you
mentioned.(i believe the bug you are talking about is if MAX is not evenly
divisible by eight i will "lose" some numbers)
/* Note: this code also has the bug, I'm going to let you find
it yourself with my hint. */

for (i = 0; i < MAX/8; ++i) {
for (j = pos; j < pos + 8; ++j) {
if (list[j] >= 0)
printf ("%-6d ", list[j]);
else
goto end_of_loop;
}

printf ("\n");
pos += 8;
}

end_of_loop:
printf("\n");

Now this approach might give your instructor a heart attack, and it
really is not very nice, but it *is* better than your double break.
-Sheldon

Lol. I am sure that it would. She seems to stick to certain style rules
rigidly (ex. the brace style you use is her style or the highway), and i
can only imagine her reaction to the use of a goto.

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

[Chris]

Thanks everyone for the insight. I didn't expect to get even half as much
feedback as i did. I think i finally decided on this bit of code:

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

It doesn't have the "not divisible by 8 bug", i removed one of the for loops,
and am down to one break statement. The only "ugly" bit is the work around
i have when i == 0. ;^)

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

[Zygmunt Krynicki]

On Thu, 06 Nov 2003 09:26:05 +0000, Chris wrote:

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

Hey, look - no breaks :-)
The code quoted with > was left unchanged from your version.

Regards
Zygmunt

[Larry Doolittle]

In article <pa****************************@yahoo.com>, Chris wrote:

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

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[i] < 0; ++i) {
if (i!=0) putchar ((i%8==0) ? '\n' : ' ');
printf ("%-4d", list[i]);
}
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]

Floyd Davidson wrote:

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

--
"As a scientist, Throckmorton knew that if he were ever to break wind in
the echo chamber, he would never hear the end of it."

[Floyd Davidson]

David Rubin <fu******@warpmail.net> wrote:

Floyd Davidson wrote:

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

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.)

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.

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.

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.

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.)

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...

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.

--
Floyd L. Davidson <http://web.newsguy.com/floyd_davidson>
Ukpeagvik (Barrow, Alaska) fl***@barrow.com

[bd]

Chris Potter wrote:

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[i] >= 0; i++){
printf("%-6d ", list[i]);
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]

ki**********@yahoo.com (Chris Potter) wrote in message news:<6d*************************@posting.google.c om>...

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[i] >= 0; i++) {
ws = (i + 1) % NPL ? ' ' : '\n';
printf("%-6d%c", list[i], ws);
}
}
Regards,
Mark Haigh
mf*****@sbcglobal.net

[David Rubin]

Floyd Davidson wrote:

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

--
Andre, a simple peasant, had only one thing on his mind as he crept
along the East wall: 'Andre, creep... Andre, creep... Andre, creep.'
-- unknown

[Floyd Davidson]

David Rubin <bo***********@nomail.com> wrote:

Floyd Davidson wrote:

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.

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.

--
Floyd L. Davidson <http://web.newsguy.com/floyd_davidson>
Ukpeagvik (Barrow, Alaska) fl***@barrow.com