Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > comp.lang.c > #25426 > unrolled thread

Request for source code review

Back to article view | Back to comp.lang.c

Contents

  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

Page 2 of 2 — ← Prev page 1 [2]

#25622

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

[toc] | [prev] | [next] | [standalone]

#25687

Ian Collins wrote:
> 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!
>

*nix "make" makefiles can easily say :
depend:
	$(CC) -M $(CFLAGS) *.c > .depend
...

include .depend


--
Les Cargill

[toc] | [prev] | [next] | [standalone]

#25715

On 08/31/2012 10:18 PM, Les Cargill wrote:
> Ian Collins wrote:
>> 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!
>>
> 
> *nix "make" makefiles can easily say :
> depend:
> 	$(CC) -M $(CFLAGS) *.c > .depend
> ...
> 
> include .depend

I tried that for several years. It ended up being more trouble than it
was worth, partly because .depend was always one version out of date
compared to the current code. Unfortunately, I don't remember the details.

What clearmake did was much more comprehensive - it tracked down all
kinds of file dependencies, not just header files, (which can be helpful
when the build script included commands other than those that invoke a
compiler) and the user never had to deal with how or where that
information was stored. As I remember, it "just worked" (at least, the
dependency tracking did - other parts of the system were a little more
complicated to deal with).
-- 
James Kuyper

[toc] | [prev] | [next] | [standalone]

#25716

James Kuyper wrote:
> On 08/31/2012 10:18 PM, Les Cargill wrote:
>> Ian Collins wrote:
>>> 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!
>>>
>>
>> *nix "make" makefiles can easily say :
>> depend:
>> 	$(CC) -M $(CFLAGS) *.c > .depend
>> ...
>>
>> include .depend
>
> I tried that for several years. It ended up being more trouble than it
> was worth, partly because .depend was always one version out of date
> compared to the current code. Unfortunately, I don't remember the details.


You only need to "make depend" when a file has its includes changed. And 
in a pinch, you can always utter "gcc -M sourcefile.c >> .depend".

>
> What clearmake did was much more comprehensive - it tracked down all
> kinds of file dependencies, not just header files, (which can be helpful
> when the build script included commands other than those that invoke a
> compiler) and the user never had to deal with how or where that
> information was stored.

I would have to use it for months before I could trust it.

> As I remember, it "just worked" (at least, the
> dependency tracking did - other parts of the system were a little more
> complicated to deal with).
>

Never used clearmake; only clearcase. Was less than impressed with
clearcase.

--
Les Cargill

[toc] | [prev] | [next] | [standalone]

#25717

On 09/ 2/12 01:45 PM, Les Cargill wrote:
> James Kuyper wrote:
>> On 08/31/2012 10:18 PM, Les Cargill wrote:
>>> Ian Collins wrote:
>>>> 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!
>>>>
>>>
>>> *nix "make" makefiles can easily say :
>>> depend:
>>> 	$(CC) -M $(CFLAGS) *.c>  .depend
>>> ...
>>>
>>> include .depend
>>
>> I tried that for several years. It ended up being more trouble than it
>> was worth, partly because .depend was always one version out of date
>> compared to the current code. Unfortunately, I don't remember the details.
>
>
> You only need to "make depend" when a file has its includes changed. And
> in a pinch, you can always utter "gcc -M sourcefile.c>>  .depend".

Sticking one instance of .KEEP_STATE: in your makefile is way more elegant.

-- 
Ian Collins

[toc] | [prev] | [next] | [standalone]

#25723

Ian Collins wrote:
> On 09/ 2/12 01:45 PM, Les Cargill wrote:
>> James Kuyper wrote:
>>> On 08/31/2012 10:18 PM, Les Cargill wrote:
>>>> Ian Collins wrote:
>>>>> 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!
>>>>>
>>>>
>>>> *nix "make" makefiles can easily say :
>>>> depend:
>>>>     $(CC) -M $(CFLAGS) *.c>  .depend
>>>> ...
>>>>
>>>> include .depend
>>>
>>> I tried that for several years. It ended up being more trouble than it
>>> was worth, partly because .depend was always one version out of date
>>> compared to the current code. Unfortunately, I don't remember the
>>> details.
>>
>>
>> You only need to "make depend" when a file has its includes changed. And
>> in a pinch, you can always utter "gcc -M sourcefile.c>>  .depend".
>
> Sticking one instance of .KEEP_STATE: in your makefile is way more elegant.
>

Neat! I'd agree.

That appears to be specific to Sun make.

--
Les Cargill

[toc] | [prev] | [next] | [standalone]

#25691

On 2012-08-30, Ian Collins <ian-news@hotmail.com> 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?

And when I use an *indecent* make, I actually don't worry about this.
Whenver I do an "important" build ("make dist", or whatever) then
automated tools do a complete rebuild.

For the tight edit-test-fix cycle,  I just try to keep the dependencies
in my head, and do a "make clean"  whenever I see a problem.  Of course
I often get it wrong, but I do a "make clean" often enough that it
doesn't get out of hand.

[toc] | [prev] | [next] | [standalone]

#25527

בתאריך יום רביעי, 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 [i]);

}


 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().

[toc] | [prev] | [next] | [standalone]

#25538

Malcolm McLean <malcolm.mclean5@btinternet.com> 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 [i]);
| }
|
|
|  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?

[toc] | [prev] | [next] | [standalone]

#25543

Udyant Wig <udyantw@gmail.com> writes:

> Malcolm McLean <malcolm.mclean5@btinternet.com> 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 [i]);
> | }
> |
> |
> |  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.

[toc] | [prev] | [next] | [standalone]

#25644

Firstly, I thank all reader

[toc] | [prev] | [next] | [standalone]

#25647

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 [i]);
}

/* 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 [i], 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 */

[toc] | [prev] | [next] | [standalone]

#25669

Udyant Wig <udyantw@gmail.com> writes:
[...]
> #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
>                                   */
[...]

The inner parentheses in the definition of LIMIT are unnecessary.
As long as MAXSUM is defined properly (which it is), you can
just write:

    #define LIMIT (MAXSUM + 1)

In general, you don't need to parenthesize single tokens
(identifiers, constants) in macro definitions.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <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"

[toc] | [prev] | [next] | [standalone]

#25692

> The inner parentheses in the definition of LIMIT are unnecessary.
> As long as MAXSUM is defined properly (which it is), you can
> just write:
>
>     #define LIMIT (MAXSUM + 1)
>
> In general, you don't need to parenthesize single tokens
> (identifiers, constants) in macro definitions.

I suspect the reason the original poster put MAXSUM in parentheses is
because he had read that macro *arguments* need to be a protected.
And that is correct!

You *do* need to parethesize arguments.  Thus you need to do

      #define MULTIPLY_THE_HARD_WAY(A, B) ( (A) * (B)  )

because otherwise  MULTIPLY_THE_HARD_WAY(x+y, 4) would expand to (x +
y*4); which would be wrong.

[toc] | [prev] | [next] | [standalone]

#25694

Adrian Ratnapala <raka@scooter.fritz.box> writes:
>> The inner parentheses in the definition of LIMIT are unnecessary.
>> As long as MAXSUM is defined properly (which it is), you can
>> just write:
>>
>>     #define LIMIT (MAXSUM + 1)
>>
>> In general, you don't need to parenthesize single tokens
>> (identifiers, constants) in macro definitions.
>
> I suspect the reason the original poster put MAXSUM in parentheses is
> because he had read that macro *arguments* need to be a protected.
> And that is correct!
>
> You *do* need to parethesize arguments.  Thus you need to do
>
>       #define MULTIPLY_THE_HARD_WAY(A, B) ( (A) * (B)  )
>
> because otherwise  MULTIPLY_THE_HARD_WAY(x+y, 4) would expand to (x +
> y*4); which would be wrong.

You're right, of course.  Single tokens in macro definitions don't
generally need to be parenthesized *unless* they're macro arguments.
Silly of me to miss that.

-- 
Keith Thompson (The_Other_Keith) kst-u@mib.org  <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"

[toc] | [prev] | [next] | [standalone]

#25682

Udyant Wig wrote:
> 
 
> /* gauss.h  begins */
> 
> #include <stdio.h>
> #include <stdlib.h>
> #include <time.h>
> 
> #include "utils.h"

As a matter of style,
my preference is to place the standard headers 
after the nonstandard header files.

I prefer to write header files in such a way 
that they do not need to have 
standard headers #included before them.

Placing the nonstandard header files first,
gives the compiler a chance 
to check that I have written them that way.

-- 
pete

[toc] | [prev] | [next] | [standalone]

#25655

On 8/29/12 21:19 , Udyant Wig wrote:

...

> 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.

Very readable code, good code comments.

One point though, AFAIK the keywords 'extern' in the header files are
not necessary.

Jan Kuiken

[toc] | [prev] | [next] | [standalone]

#25744

Jan Kuiken <jan.kuiken@quicknet.nl> writes:
| Very readable code, good code comments.
 Thank you.
 
| One point though, AFAIK the keywords 'extern' in the header files are
| not necessary.
 I was following Steve Summit's advice found in section 5.2
 "Function Prototypes" of his notes here:
    http://www.eskimo.com/~scs/cclass/notes/sx5b.html

 He mentions that  extern  is optional in function declarations, but
 that getting into the habit of writing it for all external declarations
 is an easier rule to remember.
    
| Jan Kuiken

[toc] | [prev] | [next] | [standalone]

#25753

On 9/2/12 12:09 , Udyant Wig wrote:

> | One point though, AFAIK the keywords 'extern' in the header files are
> | not necessary.

>   I was following Steve Summit's advice found in section 5.2
>   "Function Prototypes" of his notes here:
>      http://www.eskimo.com/~scs/cclass/notes/sx5b.html
>
>   He mentions that  extern  is optional in function declarations, but
>   that getting into the habit of writing it for all external declarations
>   is an easier rule to remember.

OK, I prefer to use 'extern' only when it is really necessary, as a
'heads up', something funny is going on.

Jan Kuiken

[toc] | [prev] | [standalone]

Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | comp.lang.c

csiph-web