Request for source code review

Discussion in 'C Programming' started by Udyant Wig, Aug 29, 2012.

  1. Udyant Wig

    Udyant Wig Guest

    I was following Steve Summit's C programming notes at
    http://www.eskimo.com/~scs/cclass/notes/top.html
    and got inspired by the dice-rolling project. I have written a naive
    Gaussian distribution program that simulates throwing a specific number
    of n-sided dice a given number of times. The output is the expected
    bell curve along with the numerical value of each sum's occurences.

    For example, the output for 10000 rolls of three six-sided dice is:

    3: 51
    4: 148
    5: 260
    6: 454
    7: 686
    8: 969
    9: 1146
    10: 1266
    11: 1239
    12: 1149
    13: 1013
    14: 713
    15: 433
    16: 280
    17: 152
    18: 41

    3:
    4: =
    5: ==
    6: ====
    7: ======
    8: =========
    9: ===========
    10: ============
    11: ============
    12: ===========
    13: ==========
    14: =======
    15: ====
    16: ==
    17: =
    18:


    Being a novice, I request the readers of this newsgroup to look over the
    source to suggest improvements regarding readability, layout, style,
    correctness, idioms, etc. I would appreciate it very much.

    I have placed the source files on Pastebin at this URL
    http://pastebin.com/1PTVJrTN

    In case anyone needs the Makefile, this is what I used:


    CC = clang
    FLAGS = -Wall -Wextra -std=c89 -pedantic

    gauss: gauss.o main.o utils.o
    $(CC) $(FLAGS) -o gauss gauss.o main.o utils.o
    gauss.o: gauss.c
    $(CC) $(FLAGS) -c gauss.c
    main.o: main.c
    $(CC) $(FLAGS) -c main.c
    utils.o: utils.c
    $(CC) $(FLAGS) -c utils.c

    clean:
    rm *.o gauss


    Thank you.
    Udyant Wig, Aug 29, 2012
    #1
    1. Advertising

  2. Udyant Wig

    Jorgen Grahn Guest

    On Wed, 2012-08-29, Udyant Wig wrote:
    > I was following Steve Summit's C programming notes at
    > http://www.eskimo.com/~scs/cclass/notes/top.html
    > and got inspired by the dice-rolling project. I have written a naive
    > Gaussian distribution program that simulates throwing a specific number

    ....
    > I have placed the source files on Pastebin at this URL
    > http://pastebin.com/1PTVJrTN
    >
    > In case anyone needs the Makefile, this is what I used:

    ....
    > FLAGS = -Wall -Wextra -std=c89 -pedantic

    ....
    > utils.o: utils.c
    > $(CC) $(FLAGS) -c utils.c


    Off-topic hint: if you call the flags CFLAGS instead of FLAGS, you can
    skip the object file targets. Gnu Make has builtin rules for such
    things. See the manual.

    /Jorgen

    --
    // Jorgen Grahn <grahn@ Oo o. . .
    \X/ snipabacken.se> O o .
    Jorgen Grahn, Aug 29, 2012
    #2
    1. Advertising

  3. Udyant Wig <> writes:

    > I was following Steve Summit's C programming notes at
    > http://www.eskimo.com/~scs/cclass/notes/top.html
    > and got inspired by the dice-rolling project. I have written a naive
    > Gaussian distribution program that simulates throwing a specific number
    > of n-sided dice a given number of times. The output is the expected
    > bell curve along with the numerical value of each sum's occurences.

    <snip>
    > Being a novice, I request the readers of this newsgroup to look over the
    > source to suggest improvements regarding readability, layout, style,
    > correctness, idioms, etc. I would appreciate it very much.
    >
    > I have placed the source files on Pastebin at this URL
    > http://pastebin.com/1PTVJrTN


    I think it's good.

    By the way, it's easier to comment on posted code though I know you were
    probably worried about posting a long listing here. It is longish, but
    I think it's probably within the length where people are more likely to
    read it when posted directly than when it's referenced on a sharing site.

    Anyway, I have only a few of remarks:

    * Why have the function 'd' in file on its own? If it's going to be used
    a lot other programs, it needs a much better name than 'd', and if it
    isn't, why not just put it where it is used? (Either way, 'd' is not
    really a good name for it -- too sort whatever the use).

    * It's widely regarded a good style to put parentheses round all
    macro bodies that are anything more the a constant or an identifier.
    One day you'll forget that you wrote

    #define LIMIT (MAXSUM) + 1

    and you'll write LIMIT * 2.

    * I'd just initialise my array where it is declared:

    int sums[LIMIT] = {0};

    rather than have an initsums function.

    * Finally, I prefer to avoid having extra variables. Rather than:

    int sum; /* Sum of the dice. */
    int i;

    for (i = 0; i < maxrolls; i++) {
    sum = sumdice (NDICE);
    ++sums [sum];
    }

    I'd write:

    for (i = 0; i < maxrolls; i++)
    ++sums[ sumdice(NDICE) ];

    For one thing, after a while, you run out of good names for them.
    That's already happening here: 'sumdice' is a better name than 'sum'
    i.e. the function name says more than the extra variable does.

    Anyway, it's good. Don't let my nit-picking get you down.

    --
    Ben.
    Ben Bacarisse, Aug 29, 2012
    #3
  4. Udyant Wig

    BartC Guest

    "Udyant Wig" <> wrote in message
    news:...

    > Being a novice, I request the readers of this newsgroup to look over the
    > source to suggest improvements regarding readability, layout, style,
    > correctness, idioms, etc. I would appreciate it very much.


    > http://pastebin.com/1PTVJrTN


    Your code looks beautiful.

    --
    Bartc
    BartC, Aug 29, 2012
    #4
  5. Udyant Wig <> writes:
    [...]
    > I have placed the source files on Pastebin at this URL
    > http://pastebin.com/1PTVJrTN
    >
    > In case anyone needs the Makefile, this is what I used:
    >
    >
    > CC = clang
    > FLAGS = -Wall -Wextra -std=c89 -pedantic
    >
    > gauss: gauss.o main.o utils.o
    > $(CC) $(FLAGS) -o gauss gauss.o main.o utils.o
    > gauss.o: gauss.c
    > $(CC) $(FLAGS) -c gauss.c
    > main.o: main.c
    > $(CC) $(FLAGS) -c main.c
    > utils.o: utils.c
    > $(CC) $(FLAGS) -c utils.c
    >
    > clean:
    > rm *.o gauss


    This isn't directly a C issue, but your Makefile doesn't reflect any
    dependencies on the *.h files. For example, if you edit gauss.h and
    type "make", it won't rebuild anything.

    And you might as well use "-std=c99", or even "-std=c11" if you have a
    new enough version of clang.

    --
    Keith Thompson (The_Other_Keith) <http://www.ghoti.net/~kst>
    Will write code for food.
    "We must do something. This is something. Therefore, we must do this."
    -- Antony Jay and Jonathan Lynn, "Yes Minister"
    Keith Thompson, Aug 30, 2012
    #5
  6. Udyant Wig

    Udyant Wig Guest

    Ben Bacarisse <> writes:
    | I think it's good.
    Thank you.

    | By the way, it's easier to comment on posted code though I know you were
    | probably worried about posting a long listing here. It is longish, but
    | I think it's probably within the length where people are more likely to
    | read it when posted directly than when it's referenced on a sharing site.
    I had originally wanted to post the entire source here, but I had no
    idea about the acceptable source post length.

    I will keep your advice in mind, though.

    | Anyway, I have only a few of remarks:
    | * Why have the function 'd' in file on its own? If it's going to be used
    | a lot other programs, it needs a much better name than 'd', and if it
    | isn't, why not just put it where it is used? (Either way, 'd' is not
    | really a good name for it -- too sort whatever the use).
    Makes sense.

    | * It's widely regarded a good style to put parentheses round all
    | macro bodies that are anything more the a constant or an identifier.
    | One day you'll forget that you wrote
    | #define LIMIT (MAXSUM) + 1
    |
    | and you'll write LIMIT * 2.
    I can see now how my current scheme could lead to nasty macroexpansion
    bugs when used carelessly.

    | * I'd just initialise my array where it is declared:
    |
    | int sums[LIMIT] = {0};
    |
    | rather than have an initsums function.
    I had forgotten about zero-initialization for arrays. That function of
    mine seems highly redundant now.

    | * Finally, I prefer to avoid having extra variables. Rather than:
    |
    | int sum; /* Sum of the dice. */
    | int i;
    |
    | for (i = 0; i < maxrolls; i++) {
    | sum = sumdice (NDICE);
    | ++sums [sum];
    | }
    |
    | I'd write:
    |
    | for (i = 0; i < maxrolls; i++)
    | ++sums[ sumdice(NDICE) ];
    |
    | For one thing, after a while, you run out of good names for them.
    | That's already happening here: 'sumdice' is a better name than 'sum'
    | i.e. the function name says more than the extra variable does.
    Sounds good.

    | Anyway, it's good. Don't let my nit-picking get you down.
    I think your remarks have made the code clearer to me.
    Udyant Wig, Aug 30, 2012
    #6
  7. Udyant Wig

    Udyant Wig Guest

    "BartC" <> writes:
    | Your code looks beautiful.
    Thank you.
    Udyant Wig, Aug 30, 2012
    #7
  8. Udyant Wig

    Udyant Wig Guest

    Keith Thompson <> writes:
    | This isn't directly a C issue, but your Makefile doesn't reflect any
    | dependencies on the *.h files. For example, if you edit gauss.h and
    | type "make", it won't rebuild anything.
    I will have to learn to do that, then.

    | And you might as well use "-std=c99", or even "-std=c11" if you have a
    | new enough version of clang.
    I have been focusing on ANSI C90, the language as described in K&R 2nd
    ed. Sadly, there won't be new editions of K&R to expound the later C
    standards.

    I understand that some of the useful new features of C99 include:
    * variable length arrays
    * a Boolean type
    * C++ comments
    * `for' loop variable declaration
    * struct initialization
    * the long long type
    * complex numbers

    Does there exist learning material for C99 as there does for C90?
    Or would I be better off learning C99 as a `patch' for C90?
    Udyant Wig, Aug 30, 2012
    #8
  9. On Thursday, August 30, 2012 4:36:23 AM UTC, Udyant Wig wrote:
    > Keith Thompson <> writes:
    >
    > | This isn't directly a C issue, but your Makefile doesn't reflect any
    > | dependencies on the *.h files. For example, if you edit gauss.h and
    > | type "make", it won't rebuild anything.
    >
    > I will have to learn to do that, then.
    >


    try my little Auto Makefile

    https://github.com/jhlicc/Visual-Stupid-Makefile/blob/master/Makefile
    lovecreatesbeauty, Aug 30, 2012
    #9
  10. Udyant Wig

    Ian Collins Guest

    On 08/30/12 01:55 PM, Keith Thompson wrote:
    > Udyant Wig<> writes:
    > [...]
    >> I have placed the source files on Pastebin at this URL
    >> http://pastebin.com/1PTVJrTN
    >>
    >> In case anyone needs the Makefile, this is what I used:
    >>
    >>
    >> CC = clang
    >> FLAGS = -Wall -Wextra -std=c89 -pedantic
    >>
    >> gauss: gauss.o main.o utils.o
    >> $(CC) $(FLAGS) -o gauss gauss.o main.o utils.o
    >> gauss.o: gauss.c
    >> $(CC) $(FLAGS) -c gauss.c
    >> main.o: main.c
    >> $(CC) $(FLAGS) -c main.c
    >> utils.o: utils.c
    >> $(CC) $(FLAGS) -c utils.c
    >>
    >> clean:
    >> rm *.o gauss

    >
    > This isn't directly a C issue, but your Makefile doesn't reflect any
    > dependencies on the *.h files. For example, if you edit gauss.h and
    > type "make", it won't rebuild anything.


    A decent make should provide a means for automating this. Otherwise
    where do you stop recursing up the include paths?

    --
    Ian Collins
    Ian Collins, Aug 30, 2012
    #10
  11. Udyant Wig

    James Kuyper Guest

    On 08/30/2012 01:04 AM, Ian Collins wrote:
    > On 08/30/12 01:55 PM, Keith Thompson wrote:

    ....
    >> This isn't directly a C issue, but your Makefile doesn't reflect any
    >> dependencies on the *.h files. For example, if you edit gauss.h and
    >> type "make", it won't rebuild anything.

    >
    > A decent make should provide a means for automating this. Otherwise
    > where do you stop recursing up the include paths?


    I've used a "decent" make that was part of a configuration management
    system called ClearCase, which fully automated the tracking of
    dependencies, but by rather drastic means. Whenever clearmake executed
    the build script for a given target, it would monitor the processes that
    were executed, keeping track of ALL files that were opened during the
    build. If they were versioned files managed by ClearCase, it identified
    which version of the file was currently visible; for other files it just
    looked at the file date. The next time that the same target was built,
    it would check to see whether any of the files that had been opened the
    last time had been changed, and if so, it assumed that the build script
    had to be re-executed.

    One of the key things that made this work was that the make facility was
    tightly integrated with a special device driver that managed access to
    disk space; only the use of files stored on disk volumes managed by that
    driver could be detected.

    After paying the rather hefty license fees for such sophisticated
    software, guess how it was used: whenever we delivered new code, CM
    would build it using the option that forced a complete re-build,
    regardless of dependencies.
    --
    James Kuyper
    James Kuyper, Aug 30, 2012
    #11
  12. בת×ריך ×™×•× ×¨×‘×™×¢×™, 29 ב×וגוסט 2012 20:19:06 UTC+1, מ×ת Udyant Wig:
    > I was following Steve Summit's C programming notes at
    >
    > Being a novice, I request the readers of this newsgroup to look over the
    > source to suggest improvements regarding readability, layout, style,
    > correctness, idioms, etc. I would appreciate it very much.
    >
    >

    It's a lot better than most newbie code. But you need to think a bit
    more about what makes a good function.

    d() is a good function, but a bad nmae. It rolls a ide and returns the
    result. It's intuitive, it's resuable, it represents a natural module.

    However some of the printing functions seem to exist purely to get code out
    of main(). When main() becomes so long that it's unreadable then it's
    necessary to break it down. But when the program is quite simple, it's better
    not to hide things in functions.


    This function for example is really just a wrapper for a printing loop

    /* Print the number of occurrences of the various sums. */


    void printsums (int sums [], int limit) {

    int i;


    for (i = NDICE; i < limit; i++)
    printf ("%2d: %d\n", i, sums );

    }


    then it's got another problem with it, which is that it's taking one
    limit as an argument and the other as a compile time constant. So you're
    creating confusion. Don't be afraid to pass a compile time constant as
    a parameter. Often that makes functions more understandable and more
    general.


    histsums() however is more substantial, and drawing a histogram is a natural
    unit of work. So the case for this being in a function is a lot stronger.
    You could also write a top-level fucntion printoutput(), if you want a tidy
    main().
    Malcolm McLean, Aug 30, 2012
    #12
  13. Udyant Wig <> writes:

    > Keith Thompson <> writes:
    > | This isn't directly a C issue, but your Makefile doesn't reflect any
    > | dependencies on the *.h files. For example, if you edit gauss.h and
    > | type "make", it won't rebuild anything.
    > I will have to learn to do that, then.
    >
    > | And you might as well use "-std=c99", or even "-std=c11" if you have a
    > | new enough version of clang.
    > I have been focusing on ANSI C90, the language as described in K&R 2nd
    > ed. Sadly, there won't be new editions of K&R to expound the later C
    > standards.
    >
    > I understand that some of the useful new features of C99 include:
    > * variable length arrays
    > * a Boolean type
    > * C++ comments
    > * `for' loop variable declaration
    > * struct initialization


    They are called "compound literals". "Old" C has struct initialisation,
    so that's not a good term for it. What C99 adds is the ability to
    "denote" an unnamed struct object:

    plot((struct point){3, 4});

    > * the long long type
    > * complex numbers
    >
    > Does there exist learning material for C99 as there does for C90?
    > Or would I be better off learning C99 as a `patch' for C90?


    I'd do it that way. If you find K&R's style suits you, there is
    really no better way to start and C99's extras are small.

    VLAs are the most involved, I think, and they have been made optional in
    C11. That's a crying shame to my mind, but that's how things have
    panned out. No one who's implemented VLAs will remove the support, but
    it makes them a second-class feature of the language.

    --
    Ben.
    Ben Bacarisse, Aug 30, 2012
    #13
  14. Udyant Wig

    Udyant Wig Guest

    Jorgen Grahn <> writes:
    | Off-topic hint: if you call the flags CFLAGS instead of FLAGS, you can
    | skip the object file targets. Gnu Make has builtin rules for such
    | things. See the manual.
    That worked.

    I will have to study the manual at some point.

    | /Jorgen
    Udyant Wig, Aug 30, 2012
    #14
  15. Udyant Wig

    Udyant Wig Guest

    lovecreatesbeauty <> writes:
    |> I will have to learn to do that, then.
    |>
    |
    | try my little Auto Makefile
    |
    | https://github.com/jhlicc/Visual-Stupid-Makefile/blob/master/Makefile
    That looks interesting. It should prove useful.

    It says on line 3:

    # .c and Makefile in src, .h can be in include, no other directories.

    Do the headers need to be in src or beside it?
    Udyant Wig, Aug 30, 2012
    #15
  16. Udyant Wig

    Udyant Wig Guest

    Malcolm McLean <> writes:
    | It's a lot better than most newbie code. But you need to think a bit
    | more about what makes a good function.
    |
    | d() is a good function, but a bad nmae. It rolls a ide and returns the
    | result. It's intuitive, it's resuable, it represents a natural module.
    I can think of:
    * die
    * roll_die
    * n_sided_die

    | However some of the printing functions seem to exist purely to get code out
    | of main(). When main() becomes so long that it's unreadable then it's
    | necessary to break it down. But when the program is quite simple, it's better
    | not to hide things in functions.
    |
    | This function for example is really just a wrapper for a printing loop
    |
    |
    | /* Print the number of occurrences of the various sums. */
    |
    | void printsums (int sums [], int limit) {
    |
    | int i;
    |
    | for (i = NDICE; i < limit; i++)
    | printf ("%2d: %d\n", i, sums );
    | }
    |
    |
    | then it's got another problem with it, which is that it's taking one
    | limit as an argument and the other as a compile time constant. So you're
    | creating confusion. Don't be afraid to pass a compile time constant as
    | a parameter. Often that makes functions more understandable and more
    | general.
    I have used this convention in two other functions (histogram &
    histsums). I should redesign them too.

    | histsums() however is more substantial, and drawing a histogram is a natural
    | unit of work. So the case for this being in a function is a lot stronger.
    | You could also write a top-level fucntion printoutput(), if you want a tidy
    | main().
    My aim was to make clear the flow of the program in main. Anything
    that did some specific work, I moved to its own function.

    But, I get your point.

    Incidentally, when faced with an unfamiliar codebase, where do you
    begin reading?
    Udyant Wig, Aug 30, 2012
    #16
  17. Udyant Wig <> writes:

    > Malcolm McLean <> writes:
    > | It's a lot better than most newbie code. But you need to think a bit
    > | more about what makes a good function.
    > |
    > | d() is a good function, but a bad nmae. It rolls a ide and returns the
    > | result. It's intuitive, it's resuable, it represents a natural module.
    > I can think of:
    > * die
    > * roll_die
    > * n_sided_die


    It's not really anything about dice. It picks a number in [1,n] with
    uniform probability. I'd probably go with something like uniform_int_in
    and then make the lower bound a parameter too:

    int uniform_int_in(int min, int max);

    > | However some of the printing functions seem to exist purely to get code out
    > | of main(). When main() becomes so long that it's unreadable then it's
    > | necessary to break it down. But when the program is quite simple, it's better
    > | not to hide things in functions.
    > |
    > | This function for example is really just a wrapper for a printing loop
    > |
    > |
    > | /* Print the number of occurrences of the various sums. */
    > |
    > | void printsums (int sums [], int limit) {
    > |
    > | int i;
    > |
    > | for (i = NDICE; i < limit; i++)
    > | printf ("%2d: %d\n", i, sums );
    > | }
    > |
    > |
    > | then it's got another problem with it, which is that it's taking one
    > | limit as an argument and the other as a compile time constant. So you're
    > | creating confusion. Don't be afraid to pass a compile time constant as
    > | a parameter. Often that makes functions more understandable and more
    > | general.
    > I have used this convention in two other functions (histogram &
    > histsums). I should redesign them too.
    >
    > | histsums() however is more substantial, and drawing a histogram is a natural
    > | unit of work. So the case for this being in a function is a lot stronger.
    > | You could also write a top-level fucntion printoutput(), if you want a tidy
    > | main().
    > My aim was to make clear the flow of the program in main. Anything
    > that did some specific work, I moved to its own function.


    Personally, I prefer too see too many functions rather than too few.
    Over time, laziness (if you at all like me) will cause you to get
    a better balance. It's much harder to get into the habit of hiving off
    code into functions than it is to get out of it.

    > But, I get your point.
    >
    > Incidentally, when faced with an unfamiliar codebase, where do you
    > begin reading?


    If you are lucky, the READ_ME file or the huge explanatory comment at
    the top of main.c! Failing that, I start with main() to see if I can
    get the fell of what's happening, but I will probably jump around other
    modules too just to get familiar with what is where.

    --
    Ben.
    Ben Bacarisse, Aug 30, 2012
    #17
  18. Udyant Wig

    Ian Collins Guest

    On 08/30/12 11:43 PM, James Kuyper wrote:
    > On 08/30/2012 01:04 AM, Ian Collins wrote:
    >> On 08/30/12 01:55 PM, Keith Thompson wrote:

    > ....
    >>> This isn't directly a C issue, but your Makefile doesn't reflect any
    >>> dependencies on the *.h files. For example, if you edit gauss.h and
    >>> type "make", it won't rebuild anything.

    >>
    >> A decent make should provide a means for automating this. Otherwise
    >> where do you stop recursing up the include paths?

    >
    > I've used a "decent" make that was part of a configuration management
    > system called ClearCase, which fully automated the tracking of
    > dependencies, but by rather drastic means. Whenever clearmake executed
    > the build script for a given target, it would monitor the processes that
    > were executed, keeping track of ALL files that were opened during the
    > build. If they were versioned files managed by ClearCase, it identified
    > which version of the file was currently visible; for other files it just
    > looked at the file date. The next time that the same target was built,
    > it would check to see whether any of the files that had been opened the
    > last time had been changed, and if so, it assumed that the build script
    > had to be re-executed.
    >
    > One of the key things that made this work was that the make facility was
    > tightly integrated with a special device driver that managed access to
    > disk space; only the use of files stored on disk volumes managed by that
    > driver could be detected.


    I've had hours of fun with clearmake...

    Sun's (d)make uses the output of the preprocessor phase of compiles to
    collate dependency information. Rather more elegant and a lot cheaper!

    --
    Ian Collins
    Ian Collins, Aug 31, 2012
    #18
  19. Udyant Wig

    Udyant Wig Guest

    Firstly, I thank all reader
    Udyant Wig, Aug 31, 2012
    #19
  20. Udyant Wig

    Udyant Wig Guest

    I thank all readers who responded to my request.

    I have revised the source taking into account many of the remarks made.
    Follows the new listing.

    /* utils.h begins */

    #include <stdlib.h>

    extern int uniform_int_in (int min, int max);

    /* utils.h ends */



    /* utils.c begins */

    #include "utils.h"

    /* Pick a number in [1, n] with uniform probability. */
    int
    uniform_int_in (int min, int max) {
    return min + (rand () / (RAND_MAX / max + 1));
    }

    /* utils.c ends */



    /* gauss.h begins */

    #include <stdio.h>
    #include <stdlib.h>
    #include <time.h>

    #include "utils.h"


    #define NDICE 5 /* The number of dice to be rolled. */
    #define SIDES 6 /* Each die has these many sides. */
    #define MAXSUM (SIDES * NDICE) /* The maximum possible sum of NDICE
    dice.
    */
    #define LIMIT ((MAXSUM) + 1) /* This constant is used:
    0) to declare the array of sums
    1) as a loop limit
    */
    #define ROLLS 100000 /* The number of throws of the dice */
    #define SCALE 100 /* Used to fit the histogram into the
    screen width. A lower value
    implies longer bars.
    */

    extern int roll_die (int n);
    extern int sumdice (int ndice);
    extern void fillsums (int sums [], int maxrolls);
    extern void printsums (int sums [], int begin_limit, int end_limit);
    extern void histogram (int sum, int n, int scale);
    extern void histsums (int sums [], int begin_limit, int end_limit);

    /* gauss.h ends */



    /* gauss.c begins */

    #include "gauss.h"

    /* Given n, throw an n-sided die. */
    int
    roll_die (int n) {
    return uniform_int_in (1, n);
    }


    /* Roll the given number of dice and return their sum. */
    int
    sumdice (int ndice) {
    int i;
    int sum = 0;

    for (i = 0; i < ndice; i++)
    sum += roll_die (SIDES);

    return sum;
    }

    /* For the specified number of dice throws: calculate the sum of the
    given number of dice, and increment the array slot whose index is
    the sum.
    */
    void
    fillsums (int sums [], int maxrolls) {
    int i;

    for (i = 0; i < maxrolls; i++)
    ++sums [sumdice (NDICE)];
    }

    /* Print the number of occurrences of the various sums. */
    void
    printsums (int sums [], int begin_limit, int end_limit) {
    int i;

    for (i = begin_limit; i < end_limit; i++)
    printf ("%2d: %d\n", i, sums );
    }

    /* Print a horizontal bar histogram for a given sum. */
    void
    histogram (int sum, int n, int scale) {
    int i;

    printf ("%2d: ", sum);
    for (i = 0; i < n / scale; i++)
    putchar ('=');
    putchar ('\n');
    }

    /* Print a histogram of the occurrences. */
    void
    histsums (int sums [], int begin_limit, int end_limit) {
    int i;

    if ((NDICE > 0) && (SIDES > 1)) {
    for (i = begin_limit; i < end_limit; i++)
    histogram (i, sums , SCALE);
    }
    else {
    if (NDICE < 1) {
    fputs ("NDICE set to insane value\n", stderr);
    fputs ("So, no histogram.\n", stderr);
    }
    if (SIDES < 2) {
    fputs ("SIDES set to insane value\n", stderr);
    fputs ("So, no histogram.\n", stderr);
    }
    }
    }

    /* gauss.c ends */



    /* main.c begins */

    #include "gauss.h"

    int
    main (void) {
    int sums [LIMIT] = {0}; /* We use only indices
    NDICE to LIMIT - 1
    for indexing the sums of
    the given of dice. We waste
    sizeof (int) * NDICE
    bytes.
    */

    srand (time (NULL)); /* Seed the RNG using the
    current time.
    */
    fillsums (sums, ROLLS); /* Count the sums. */
    printsums (sums, NDICE, LIMIT); /* Print how many of each sum
    counted.
    */
    putchar ('\n');
    histsums (sums, NDICE, LIMIT); /* Print a histogram of
    the same.
    */

    exit (EXIT_SUCCESS); /* Done. So, exit. */
    }

    /* main.c ends */
    Udyant Wig, Aug 31, 2012
    #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. Code Review Request

    , Jun 30, 2005, in forum: Java
    Replies:
    2
    Views:
    508
    Andrew Thompson
    Jun 30, 2005
  2. P Kenter

    Request for code review

    P Kenter, May 28, 2004, in forum: C++
    Replies:
    4
    Views:
    353
    P Kenter
    Jun 2, 2004
  3. Ben Hanson

    Request for Code Review

    Ben Hanson, Jul 2, 2004, in forum: C++
    Replies:
    19
    Views:
    653
    Chris Gordon-Smith
    Jul 4, 2004
  4. www
    Replies:
    51
    Views:
    1,454
  5. Udyant Wig
    Replies:
    88
    Views:
    256
    Malcolm McLean
    Apr 21, 2014
Loading...

Share This Page