«

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.

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 .

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.

"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

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) kst- <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"

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.

"BartC" <> writes:
| Your code looks beautiful.
Thank you.

Keith Thompson <kst-> 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?

On Thursday, August 30, 2012 4:36:23 AM UTC, Udyant Wig wrote:
> Keith Thompson <kst-> 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-Stu...aster/Makefile

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