Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > An "acceptable" use of break

Reply
Thread Tools

An "acceptable" use of break

 
 
Sheldon Simms
Guest
Posts: n/a
 
      11-06-2003
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


 
Reply With Quote
 
 
 
 
Andy
Guest
Posts: n/a
 
      11-06-2003
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.


 
Reply With Quote
 
 
 
 
Andy
Guest
Posts: n/a
 
      11-06-2003
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.


 
Reply With Quote
 
Andy
Guest
Posts: n/a
 
      11-06-2003
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
 
Reply With Quote
 
Sheldon Simms
Guest
Posts: n/a
 
      11-06-2003
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...

 
Reply With Quote
 
Christian Bau
Guest
Posts: n/a
 
      11-06-2003
In article <(E-Mail Removed)> ,
http://www.velocityreviews.com/forums/(E-Mail Removed) (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 += {
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 == {
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.
 
Reply With Quote
 
Christian Bau
Guest
Posts: n/a
 
      11-06-2003
In article <2ujqb.62662$(E-Mail Removed)>,
David Rubin <(E-Mail Removed)> 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.
 
Reply With Quote
 
Chris
Guest
Posts: n/a
 
      11-06-2003
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"

 
Reply With Quote
 
Chris
Guest
Posts: n/a
 
      11-06-2003
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"

 
Reply With Quote
 
Zygmunt Krynicki
Guest
Posts: n/a
 
      11-06-2003
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
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
`if (!p ? i++ : 0) break;' == `if (!p){ i++; break;}' ? lovecreatesbea...@gmail.com C Programming 12 04-14-2008 07:59 AM
Perl loops should use break, not last Jeremy Morton Perl 1 01-30-2005 10:50 PM
Do not use SAFEBOOT SOLO from CONTROL BREAK INTERNATIONAL mncvklsdnfoesdjhfgod Guy Domville Computer Security 48 08-03-2004 07:01 AM
Do not use SAFEBOOT SOLO from CONTROL BREAK INTERNATIONAL vlfdjgdsjgdfjghfg A.Melon Computer Security 8 07-14-2004 08:00 PM
SAFEBOOT SOLO from CONTROL BREAK INTERNATIONAL: DO NOT USE hwefoech9hfcvkbvi Fritz Wuehler Computer Security 6 07-02-2004 03:07 PM



Advertisments