coin toss problem

C

celerysoup16

I've written this coin toss program, and can't figure out why it isn't
giving accurate results...
cheers,
Ben

#include <stdlib.h>
#include <stdio.h>
#define H 1
#define T 0
#define SENTINEL -1

int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;

void one_run(int seed_passed)
{
int i, x, row_h;
row_h = 0;

seed=(1+seed_passed);
srand(seed);

char A[limit+1];

for(i=0;i<limit;i++)
{
x=(int)(2.0*rand()/(1.0+RAND_MAX));

if(x==H)
A=H;

else
A=T;
}

A=SENTINEL;

i=0;

while(A!=SENTINEL)
{
while(A==H)
{
i++;
row_h++;
}

if(row_h==in_a_row)
count++;

row_h=0;

if(A!=SENTINEL)
i++;
}


}

int main(int argc, char **argv)
{
int i;
double av_occur;

if (argc==2)
seed=atoi(argv[1]);

printf("\nEnter number of coin tosses: ");
scanf("%d", &limit);

printf("\nEnter number of heads to be found in a row: ");
scanf("%d", &in_a_row);

if(limit<=1000)
iterations=100000;

for(i=1;i<=iterations;i++)
{
one_run(seed);
}

(av_occur=1.0*count/iterations);

printf("\nIf a coin is flipped %d times:\n", limit);
printf("Average no. of occurances of %d heads in a row is %.5f"
, in_a_row, av_occur);
printf("\n(with %d tests)", iterations);

/* printf("\ncount is %d", count);
*/
return 0;
}
 
C

CBFalconer

I've written this coin toss program, and can't figure out why it isn't
giving accurate results...
cheers,
Ben

#include <stdlib.h>
#include <stdio.h>
#define H 1
#define T 0
#define SENTINEL -1

int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;

void one_run(int seed_passed)
{
int i, x, row_h;
row_h = 0;

seed=(1+seed_passed);
srand(seed);

char A[limit+1];

This code should not compile unless you have a C99 system, which I
doubt. limit+1 is not a constant, so the compiler has no idea what
to do. In addition, you are defining storage after executable
code. Turn up your warning levels. Also you are allowed to use
blanks in expressions, the blank embargo was lifted some years ago.
 
I

Ian Collins

CBFalconer said:
I've written this coin toss program, and can't figure out why it isn't
giving accurate results...
cheers,
Ben

#include <stdlib.h>
#include <stdio.h>
#define H 1
#define T 0
#define SENTINEL -1

int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;

void one_run(int seed_passed)
{
int i, x, row_h;
row_h = 0;

seed=(1+seed_passed);
srand(seed);

char A[limit+1];


This code should not compile unless you have a C99 system, which I
doubt.

Why? Compiles fine with either compiler on mine.
 
O

osmium

I've written this coin toss program, and can't figure out why it isn't
giving accurate results...

I don't know what your problem is but I will make some observations anyway.
First of all, get rid of the global variables. There is no need to resort
to global variables in a program this small and simple.

Is there any reason not to use enum instead of the #defines? They are more
appropriate and would probably do what you want.

Don't use the sentinel. Sentinels are something you resort to when there is
a good reason. There is no good reason here.

Reduce the program to its essence. You seem to be able to handle command
line, a default set of data, or user supplied data. This is just
obfuscating things right now. Choose one. I would use built in data for
initial testing.

Pay attention to the definition of in_a_row. Does it mean *exactly* n in a
row? The code says no. To determine that your code would have to look at
the next character - the one that breaks the sequence- , and it doesn't do
that. To me, exactly is the most sensible thing to compute. For example,
if in_a_row is 3 what does the following sequence of heads get counted as:
001111111100 ?

I tried the corner cases, in a row = 1 and in a row = 0 and got strange
results.

You might refuse to accept such questions as n = 1 in the final version of
your code.

The crucial number is count. If you still have problems, add code to print
the raw value of count.

There are also a few embedded comments.
#include <stdlib.h>
#include <stdio.h>
#define H 1
#define T 0
#define SENTINEL -1

int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;

void one_run(int seed_passed)
{
int i, x, row_h;
row_h = 0;

Don't separate the definition from the initial value.

IOW
int row_h = 0;
seed=(1+seed_passed);
srand(seed);

char A[limit+1];

for(i=0;i<limit;i++)
{
x=(int)(2.0*rand()/(1.0+RAND_MAX));

Too complicated.

Note that if the number drawn is less than 1/2 RAND_MAX, you have divided
the distribution into two almost equal parts.
if(x==H)
A=H;

else
A=T;
}

A=SENTINEL;

i=0;

while(A!=SENTINEL)
{
while(A==H)
{
i++;
row_h++;
}

if(row_h==in_a_row)
count++;

row_h=0;

if(A!=SENTINEL)
i++;
}


}

int main(int argc, char **argv)
{
int i;
double av_occur;

if (argc==2)
seed=atoi(argv[1]);

printf("\nEnter number of coin tosses: ");
scanf("%d", &limit);

printf("\nEnter number of heads to be found in a row: ");
scanf("%d", &in_a_row);

if(limit<=1000)
iterations=100000;

for(i=1;i<=iterations;i++)
{
one_run(seed);
}

(av_occur=1.0*count/iterations);


What are those parens for? Just a kind of idle question.
 
R

Richard Heathfield

Ian Collins said:
CBFalconer said:
(e-mail address removed) wrote:
int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;
char A[limit+1];


This code should not compile unless you have a C99 system, which I
doubt.

Why? Compiles fine with either compiler on mine.

If you're using a C99 compiler, well, fair enough. If you're using a
compiler adhering to an older standard, like 99.90813774% of us, then your
C compiler is required to issue a diagnostic message for the above code,
which violates this constraint: "The expression that specifies the size of
an array shall be an integral constant expression that has a value greater
than zero." Since limit+1 is not an integral constant expression, a
diagnostic message is required.
 
B

Bill Pursell

osmium said:
Don't separate the definition from the initial value.

IOW
int row_h = 0;

Is there a reason to use an initialization instead of
an assignment after the declaration? I prefer declaring
and then assigning, but the only reason is that I find
it annoying when I'm debugging and the debugger stops
in the declaration block. Is this more than just a stylistic
issue, or do some compilers generate different code for
the 2 cases?
 
O

osmium

Bill Pursell said:
Is there a reason to use an initialization instead of
an assignment after the declaration? I prefer declaring
and then assigning, but the only reason is that I find
it annoying when I'm debugging and the debugger stops
in the declaration block. Is this more than just a stylistic
issue, or do some compilers generate different code for
the 2 cases?

My objection was based on documentation. I found a place in his code where
the value seemed to be an un-initialized datum. In fact it wasn't, but the
initialization was hidden, at least from me. For a while.

I doubt if there are any non-stylistic issues.
 
J

Joe Smith

cheers,
Ben
#include <stdlib.h>
#include <stdio.h>
#define H 1
#define T 0
#define SENTINEL -1
I might be able to provide comment on your data, but I crap out right here.
Unlike Keith Thompson, I am not 100% certain what this ultimate statement
means. BTW, I'm not even sure that 'statement" is the right descriptor for
something that doesn't necessarily end in a ';'. Anyways,
#define SENTINEL -1
probably instructs the preprocessor to replace every instance of SENTINEL
with negative one. When I looked at it, I thought you were decrementing. I
think better form would be
#define SENTINEL '-1'
or
#define SENTINEL (-1)
, I'm not sure which, but since '-' is a token, it needs something. Post
your "howdy and thanks, ben" below, so that they can be easily snipped.
furunculus
 
P

pete

Bill said:
Is there a reason to use an initialization instead of
an assignment after the declaration? I prefer declaring
and then assigning,

So do I.
but the only reason is that I find
it annoying when I'm debugging and the debugger stops
in the declaration block.

I prefer to initialise just prior to usage because
my compiler issues a warning for unused variables,
which is a warning that I like.
My compiler considers an initialised variable,
to be a used one.
 
P

pete

I think better form would be
#define SENTINEL '-1'
or
#define SENTINEL (-1)
, I'm not sure which, but since '-' is a token, it needs something.

I prefer
#define SENTINEL (-1)
because I don't know what '-1' means.
 
C

CBFalconer

osmium said:
:
.... snip ...

My objection was based on documentation. I found a place in his
code where the value seemed to be an un-initialized datum. In
fact it wasn't, but the initialization was hidden, at least from
me. For a while.

I doubt if there are any non-stylistic issues.

IMO the opposite is true. Since initialization of automatic
variables always involves generating code, lets have that code
where it is obviously such. If the routine is so long and involved
that you can miss the actual initialization, then it should be
chopped up into smaller more understandable portions. That way you
also avoid generating useless code for initializations that are
always just overwritten.

One further benefit: if you port your code to some C-like embedded
system with an almost C compiler, it has a better chance of
surviving unscathed.
 
I

Ian Collins

Richard said:
Ian Collins said:

CBFalconer said:
(e-mail address removed) wrote:
int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;
char A[limit+1];


This code should not compile unless you have a C99 system, which I
doubt.

Why? Compiles fine with either compiler on mine.


If you're using a C99 compiler, well, fair enough. If you're using a
compiler adhering to an older standard, like 99.90813774% of us, then your
C compiler is required to issue a diagnostic message for the above code,
which violates this constraint: "The expression that specifies the size of
an array shall be an integral constant expression that has a value greater
than zero." Since limit+1 is not an integral constant expression, a
diagnostic message is required.
Are Solaris users realy that small a minority?

Also gcc has supported variable length arrays as an extension for many
years. I've suffered the pain of porting from gcc more than once.
 
K

Keith Thompson

Richard Heathfield said:
Ian Collins said:
CBFalconer said:
(e-mail address removed) wrote:
int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;
char A[limit+1];


This code should not compile unless you have a C99 system, which I
doubt.

Why? Compiles fine with either compiler on mine.

If you're using a C99 compiler, well, fair enough. If you're using a
compiler adhering to an older standard, like 99.90813774% of us, then your
C compiler is required to issue a diagnostic message for the above code,
which violates this constraint: "The expression that specifies the size of
an array shall be an integral constant expression that has a value greater
than zero." Since limit+1 is not an integral constant expression, a
diagnostic message is required.

It also has declarations following statements within a block. Both of
these features are supported by C99 but not by C90. (They're also
supported by C++, and possibly by some C90 compilers in a
non-conforming mode.)
 
K

Keith Thompson

pete said:
I prefer
#define SENTINEL (-1)
because I don't know what '-1' means.

I know what '-1' means. It means an implementation-defined value of
type int. It's unlikely to be what you want here. (Actually it will
probably work as long as the value is neither 0 nor 1, but it's a
silly way to do it.)

If you want to define a macro with the value -1, you should definitely
add parentheses:

#define SENTINEL (-1)

to avoid operator precedence problems.

Joe, your suggestion of

#define SENTINEL '-1'

was obviously a wild guess. That's fine if you're asking a question,
but I suggest that it's counterproductive if you're trying to answer a
question.
 
I

Ian Collins

Keith said:
Richard Heathfield said:
Ian Collins said:

CBFalconer wrote:

(e-mail address removed) wrote:

int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;

char A[limit+1];


This code should not compile unless you have a C99 system, which I
doubt.

Why? Compiles fine with either compiler on mine.

If you're using a C99 compiler, well, fair enough. If you're using a
compiler adhering to an older standard, like 99.90813774% of us, then your
C compiler is required to issue a diagnostic message for the above code,
which violates this constraint: "The expression that specifies the size of
an array shall be an integral constant expression that has a value greater
than zero." Since limit+1 is not an integral constant expression, a
diagnostic message is required.


It also has declarations following statements within a block. Both of
these features are supported by C99 but not by C90. (They're also
supported by C++, and possibly by some C90 compilers in a
non-conforming mode.)
No, VLAs are not supported by C++ (yet). A constant expression is
required. g++ has them as an (irritating for porters) extension.
 
R

Richard Heathfield

Ian Collins said:
Richard said:
Ian Collins said:

CBFalconer wrote:

(e-mail address removed) wrote:

int count=0, seed=3645, limit=200, in_a_row=6, iterations=100;

char A[limit+1];


This code should not compile unless you have a C99 system, which I
doubt.

Why? Compiles fine with either compiler on mine.


If you're using a C99 compiler, well, fair enough. If you're using a
compiler adhering to an older standard, like 99.90813774% of us, then
your C compiler is required to issue a diagnostic message for the above
code, which violates this constraint: "The expression that specifies the
size of an array shall be an integral constant expression that has a
value greater than zero." Since limit+1 is not an integral constant
expression, a diagnostic message is required.
Are Solaris users realy that small a minority?

The 99.90813774% figure makes up in precision what it lacks in accuracy. :)
But seriously, if Solaris is relevant to your point then your point is
almost certainly irrelevant to comp.lang.c. Yes, the code is valid C99, as
I already pointed out in my earlier reply, so if you are using a C99
compiler then the code is legal. But it is not /portable/ - for example, it
won't compile on the implementation I am presently using.

Also gcc has supported variable length arrays as an extension for many
years.

Using extensions is fine until the time comes to port. At that point, the
pain hits. If you want to avoid the pain, either avoid the port or avoid
the extensions.
I've suffered the pain of porting from gcc more than once.

See?
 
I

Ian Collins

Richard said:
Ian Collins said:




The 99.90813774% figure makes up in precision what it lacks in accuracy. :)
But seriously, if Solaris is relevant to your point then your point is
almost certainly irrelevant to comp.lang.c. Yes, the code is valid C99, as
I already pointed out in my earlier reply, so if you are using a C99
compiler then the code is legal. But it is not /portable/ - for example, it
won't compile on the implementation I am presently using.
My poorly made point was that C99 is more widely available than some
people make out.

I still find the "it's new so I won't use it" argument hard to swallow.
Every new tool or technology has its early adopters (if 6 years on is
early!). If no one uses new stuff because it isn't portable, progress
would grind to a halt.
 
E

Eric Sosman

Keith said:
I know what '-1' means. It means an implementation-defined value of
type int. It's unlikely to be what you want here. (Actually it will
probably work as long as the value is neither 0 nor 1, but it's a
silly way to do it.)

If you want to define a macro with the value -1, you should definitely
add parentheses:

#define SENTINEL (-1)

to avoid operator precedence problems.

For example,

int array[] = { 1, 2, 3 }, *p = array + 1;
printf ("%d\n", SENTINEL[p]);

.... outputs "-3\n" if SENTINEL is -1, but "1\n" if it
is (-1).

(I *think* the only way to form an expression where
the presence or absence of parentheses around -1 makes
a difference is to use the bass-ackward array notation
as above. Can anyone come up with another?)
 
K

Keith Thompson

Ian Collins said:
Keith Thompson wrote: [...]
It also has declarations following statements within a block. Both of
these features are supported by C99 but not by C90. (They're also
supported by C++, and possibly by some C90 compilers in a
non-conforming mode.)
No, VLAs are not supported by C++ (yet). A constant expression is
required. g++ has them as an (irritating for porters) extension.

See, that's what I get for discussing C++ in comp.lang.c!

(I was probably thinking of the fact that C++ has a looser definition
of constant expressions than C has; some things that would be VLAs in
C are ordinary constant-size arrays in C++.)
 

Ask a Question

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

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
474,184
Messages
2,570,973
Members
47,529
Latest member
JaclynShum

Latest Threads

Top