Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
| From | Udyant Wig <udyantw@gmail.com> |
|---|---|
| Newsgroups | comp.lang.c |
| Subject | Re: Request for source code review |
| Date | 2012-08-30 09:44 +0530 |
| Organization | A noiseless patient Spider |
| Message-ID | <876281ggnb.fsf@panda.goosenet.in> (permalink) |
| References | <87bohty09k.fsf@panda.goosenet.in> <0.d7d1accdadfb8b47e6d9.20120829222744BST.87ipc1e6cv.fsf@bsb.me.uk> |
Ben Bacarisse <ben.usenet@bsb.me.uk> 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.
Back to comp.lang.c | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-30 00:49 +0530
Re: Request for source code review Jorgen Grahn <grahn+nntp@snipabacken.se> - 2012-08-29 19:43 +0000
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-30 18:46 +0530
Re: Request for source code review Ben Bacarisse <ben.usenet@bsb.me.uk> - 2012-08-29 22:27 +0100
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-30 09:44 +0530
Re: Request for source code review Seungbeom Kim <musiphil@bawi.org> - 2012-09-07 17:31 -0700
Re: Request for source code review Ian Collins <ian-news@hotmail.com> - 2012-09-08 12:41 +1200
Re: Request for source code review Ben Bacarisse <ben.usenet@bsb.me.uk> - 2012-09-08 04:06 +0100
Re: Request for source code review 88888 Dihedral <dihedral88888@googlemail.com> - 2012-09-07 23:50 -0700
Re: Request for source code review Ben Bacarisse <ben.usenet@bsb.me.uk> - 2012-09-08 12:28 +0100
Re: Request for source code review "BartC" <bc@freeuk.com> - 2012-08-29 22:58 +0100
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-30 09:48 +0530
Re: Request for source code review Keith Thompson <kst-u@mib.org> - 2012-08-29 18:55 -0700
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-30 10:06 +0530
Re: Request for source code review lovecreatesbeauty <lovecreatesbeauty@gmail.com> - 2012-08-29 21:59 -0700
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-30 18:51 +0530
Re: Request for source code review Ben Bacarisse <ben.usenet@bsb.me.uk> - 2012-08-30 13:44 +0100
Re: Request for source code review lovecreatesbeauty <lovecreatesbeauty@gmail.com> - 2012-09-12 03:24 -0700
Re: Request for source code review Ian Collins <ian-news@hotmail.com> - 2012-08-30 17:04 +1200
Re: Request for source code review James Kuyper <jameskuyper@verizon.net> - 2012-08-30 07:43 -0400
Re: Request for source code review Ian Collins <ian-news@hotmail.com> - 2012-08-31 20:43 +1200
Re: Request for source code review Les Cargill <lcargill99@comcast.com> - 2012-08-31 21:18 -0500
Re: Request for source code review James Kuyper <jameskuyper@verizon.net> - 2012-09-01 20:34 -0400
Re: Request for source code review Les Cargill <lcargill99@comcast.com> - 2012-09-01 20:45 -0500
Re: Request for source code review Ian Collins <ian-news@hotmail.com> - 2012-09-02 15:11 +1200
Re: Request for source code review Les Cargill <lcargill99@comcast.com> - 2012-09-01 23:43 -0500
Re: Request for source code review Adrian Ratnapala <raka@scooter.fritz.box> - 2012-09-01 07:40 +0000
Re: Request for source code review Malcolm McLean <malcolm.mclean5@btinternet.com> - 2012-08-30 05:10 -0700
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-30 19:14 +0530
Re: Request for source code review Ben Bacarisse <ben.usenet@bsb.me.uk> - 2012-08-30 16:42 +0100
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-31 19:44 +0530
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-08-31 20:03 +0530
Re: Request for source code review Keith Thompson <kst-u@mib.org> - 2012-08-31 13:34 -0700
Re: Request for source code review Adrian Ratnapala <raka@scooter.fritz.box> - 2012-09-01 07:49 +0000
Re: Request for source code review Keith Thompson <kst-u@mib.org> - 2012-09-01 02:53 -0700
Re: Request for source code review pete <pfiland@mindspring.com> - 2012-08-31 21:15 -0400
Re: Request for source code review Jan Kuiken <jan.kuiken@quicknet.nl> - 2012-08-31 19:55 +0200
Re: Request for source code review Udyant Wig <udyantw@gmail.com> - 2012-09-02 15:39 +0530
Re: Request for source code review Jan Kuiken <jan.kuiken@quicknet.nl> - 2012-09-02 19:57 +0200
csiph-web