An "acceptable" use of break

Discussion in 'C Programming' started by Chris Potter, Nov 6, 2003.

  1. Chris Potter

    Chris Potter Guest

    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"
     
    Chris Potter, Nov 6, 2003
    #1
    1. Advertising

  2. Chris Potter

    Alex Guest

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


    How about:

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

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


    Alex
     
    Alex, Nov 6, 2003
    #2
    1. Advertising

  3. Chris Potter

    Sidney Cadot Guest

    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
     
    Sidney Cadot, Nov 6, 2003
    #3
  4. Chris Potter

    Andy Guest

    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');
    }
     
    Andy, Nov 6, 2003
    #4
  5. (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 < 0) break;
    printf ("%c%-6d", (i & NL_MASK) ? ' ' : '\n', list);
    }
    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 < 0) break;
    printf ("%c%-6d", j ? ' ' : '\n', list);
    if (NL_MASK == j) j = -1;
    }
    printf ("\n");

    --
    Floyd L. Davidson <http://web.newsguy.com/floyd_davidson>
    Ukpeagvik (Barrow, Alaska)
     
    Floyd Davidson, Nov 6, 2003
    #5
  6. Chris Potter

    Chris Guest

    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"
     
    Chris, Nov 6, 2003
    #6
  7. Chris Potter

    David Rubin Guest

    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, Nov 6, 2003
    #7
  8. Chris Potter

    David Rubin Guest

    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 < 0) break;
    > printf ("%c%-6d", (i & NL_MASK) ? ' ' : '\n', list);
    > }
    > 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 >= 0; ++i){
    printf("%d", list);
    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."
     
    David Rubin, Nov 6, 2003
    #8
  9. 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 >= 0)
    {
    printf("%-6d", list);

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

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

    -Sheldon
     
    Sheldon Simms, Nov 6, 2003
    #9
  10. David Rubin <> 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 < 0) break;
    >> printf ("%c%-6d", (i & NL_MASK) ? ' ' : '\n', list);
    >> }
    >> 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 < 0) break;
    printf ("%c%-6d", (i % NL_MASK) ? ' ' : '\n', list);
    }

    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 >= 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, 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)
     
    Floyd Davidson, Nov 6, 2003
    #10
  11. 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
     
    Sheldon Simms, Nov 6, 2003
    #11
  12. Chris Potter

    Andy Guest

    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, Nov 6, 2003
    #12
  13. Chris Potter

    Andy Guest

    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.
     
    Andy, Nov 6, 2003
    #13
  14. Chris Potter

    Andy Guest

    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
     
    Andy, Nov 6, 2003
    #14
  15. 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...
     
    Sheldon Simms, Nov 6, 2003
    #15
  16. In article <>,
    (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, Nov 6, 2003
    #16
  17. In article <2ujqb.62662$>,
    David Rubin <> 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 < 0) break;
    > > printf ("%c%-6d", (i & NL_MASK) ? ' ' : '\n', list);
    > > }
    > > 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.
     
    Christian Bau, Nov 6, 2003
    #17
  18. Chris Potter

    Chris Guest

    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, Nov 6, 2003
    #18
  19. Chris Potter

    Chris Guest

    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 < 0)
    break;
    if (i % 8 != 0)
    printf ("%-4d ", list);
    else if (i == 0)
    printf ("%-4d ", list);
    else
    printf ("%-4d\n", list);
    }

    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"
     
    Chris, Nov 6, 2003
    #19
  20. 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 >= 0; ++i) {
    if (i % 8 != 0 || i == 0)
    printf ("%-4d ", list);
    > else
    > printf ("%-4d\n", list);
    > }


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

    Regards
    Zygmunt
     
    Zygmunt Krynicki, Nov 6, 2003
    #20
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. valentin tihomirov

    Is this trick with reset acceptable?

    valentin tihomirov, Apr 11, 2004, in forum: VHDL
    Replies:
    6
    Views:
    564
    Jim Lewis
    Apr 14, 2004
  2. .Net Sports
    Replies:
    2
    Views:
    903
    Craig Deelsnyder
    May 3, 2005
  3. steve

    acceptable way to program

    steve, Dec 31, 2004, in forum: Java
    Replies:
    34
    Views:
    1,035
    Haximus
    Feb 20, 2005
  4. myheartinamerica

    acceptable use of goto?

    myheartinamerica, Mar 20, 2008, in forum: C Programming
    Replies:
    54
    Views:
    1,394
    Bartc
    Mar 27, 2008
  5. Replies:
    12
    Views:
    969
Loading...

Share This Page