C++ Coin flip simulator and data collector

Question

Asks the user for the chance of a coin landing on heads, the number of trials per experiment, and the number of experiments. For example, given 5 trials per experiment and 20 experiments, the program will flip a coin 5 times and record the results 20 times. Then, it displays the results, as well as the theoretical and observed probabilities of each event happening.

I turned this in already but I really enjoyed doing the assignment and I'd like to know how to make it better, faster, etc. I'm pretty new to computer science so I'd like to learn the correct practices and everything. Right off the bat I know that I should pass things by reference instead of by value and that I shouldn't have used global variables.

//Name Lastname
//Class
//Coin Flip Simulation
//LIBRARIES
#include <stdio.h>
#include <iostream>
#include <fstream>
#include <time.h>
#include <cmath>
using namespace std;
//FUNCTION PROTOTYPES
bool generate(); //Performs a coin flip with the given chance
void runExperiment(); //Performs and prints the results of the trials
void getInfo(); //Gets the information to perform the experiment
int choose(int, int); //Performs a combination (nCk)
double probability(int); //Calculates the probability of a coin hitting heads n times (binomial thrm)
//GLOBAL VARIABLES
double chance = 0.5; //Chance of landing on heads
int numTrials = 5; //Number of trials per experiment
int numExperiments = 1000; //Number of experiments
bool printT = false; //Whether to print the results of each trial
int main()
{
 srand(time(NULL));
 getInfo();
 runExperiment();
 getchar();
 getchar();
 return 0;
}
void getInfo()
{
 cout << "Please enter the chance of landing on heads (between 0.0 and 1.0, recommended 0.5)" << endl;
 cin >> chance;
 cout << "Please enter the number of trials per experiment (max 20, recommended 5)" << endl;
 cin >> numTrials;
 cout << "Please enter the number of experiments (recommended: 1000, max 1 billion)" << endl;
 cin >> numExperiments;
 cout << "Print the results of each experiment? Takes much longer (0 for no, 1 for yes)" << endl;
 cin >> printT;
}
void runExperiment()
{
 int m = 0, n = 0, a[22];
 for (int b = 0; b < 22; b++)
 a[b] = 0;
 for (int i = 0; i < numExperiments; i++)
 {
 int k = 0, l = 0;
 for (int j = 0; j < numTrials; j++)
 {
 if (generate())
 k++;
 else
 l++;
 }
 if (printT) //Prints experiment results
 {
 cout << "TRIAL: NUMBER OF HEADS: " << k;
 cout << " NUMBER OF TAILS: " << l << endl;
 }
 for (int p = 0; p < numTrials+1; p++)
 {
 if (k == p)
 a[p]++;
 }
 m = m + k;
 n = n + l;
 }
 //Prints final information to screen
 cout << "-----INFORMATION-----" << endl;
 cout << numTrials << " TRIALS PER EXPERIMENT" << endl;
 cout << numExperiments << " EXPERIMENTS" << endl;
 cout << "CHANCE OF LANDING ON HEADS: " << chance * 100 << "%" << endl;
 cout << endl << "-----RESULTS-----" << endl;
 cout << "TOTAL HEADS: " << m << endl;
 cout << "TOTAL TAILS: " << n << endl;
 cout << "PERCENT OF TIMES THE COIN LANDED ON HEADS: " << ((double)m / (m + n))*100 << "%" << endl;
 for (int q = 0; q < numTrials+1; q++)
 cout << "NUMBER OF EXPERIMENTS WITH " << q << " HEADS AND " << numTrials - q << " TAILS: " << a[q] << endl;
 cout << "-----STATS-----" << endl;
 double summation = 0;
 for (int z = 0; z < numTrials + 1; z++)
 summation = summation + a[z];
 for (int q = 0; q < numTrials + 1; q++)
 cout << "OBSERVED PROBABILITY OF " << q << " HEADS AND " << numTrials - q << " TAILS: " << (a[q]/summation)*100 << "%" << endl;
 cout << endl;
 for (int q = 0; q < numTrials + 1; q++)
 cout << "THEORETICAL PROBABILITY OF " << q << " HEADS AND " << numTrials - q << " TAILS: " << probability(q) << "%" << endl;
}
double probability(int k) //Binomial thrm
{
 double f = choose(numTrials, k)*(pow((chance), k))*(pow((1-chance), (numTrials - k)));
 f = f * 100;
 return f;
}
int choose(int n, int k) //nCk
{
 if (k > n)
 return 0;
 int r = 1;
 for (int d = 1; d <= k; ++d)
 {
 r *= n--;
 r /= d;
 }
 return r;
}
bool generate()
{
 double i = rand() % 100;
 i = i / 100;
 if (i < chance)
 return true; //HEADS
 else
 return false; //TAILS
}

• Loki Astari: rand() is known for its non randomness in the last bit. Also since C++14 it has been replaced by a better random number library. Also the random range is not exactly dividable by 100. So numbers from [0-68] are slightly more likely than number [69-99] (assuming random max is 32768)
• kyrill: @LokiAstari That's arguably not his greatest concern...

Answer 1

There's a lot that can be improved here, so I hope that these suggestions are useful to you.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid.

Make sure you have all required #includes

The code uses rand() but doesn't #include <cstdlib>. It's important to make sure you have all required include files to make sure your program compiles reliably.

Use only required #includes

The inverse of the above suggestion is to avoid having extra #includes. In this case <fstream> does not appear to be used. Only include files that are actually needed.

Avoid the use of global variables

As you have already noted, global variables should be avoided in favor of local variables. It's generally better to explicitly pass variables your function will need rather than using the vague implicit linkage of a global variable. For example, the runExperiment function relies on numExperiment and numTrials, so those should be passed parameters.

Use better naming

The variables numTrials and numExperiments are good names because they hint at the significance of those variables to this program. However, the variables in runExperiment are named m, n, a, b, etc. which are not at all descriptive. Similarly runExperiment is a decent function name but generate and getInfo are not (generate what?; get what info?).

Use a better random number generator

The way you're using rand() could be greatly improved. Instead of your existing code for generate, you could use this:

bool generate() {
 return rand() < chance*RAND_MAX;
}

The advantage is that it does not skew the results as mentioned in a comment. Better still, use std::bernoulli_distribution which uses the modern C++ random number generation capabilities.

Use objects

The current code is largely just plain procedural C rather than C++. I would recommend wrapping all of the global variables into an object, say CoinToss and then making most of the functions member functions of that object.

Don't use std::endl when '\n' will do

Using std::endl emits a \n and flushes the stream. Unless you really need the stream flushed, you can improve the performance of the code by simply emitting '\n' instead of using the potentially more computationally costly std::endl.

Sanitize user input better

If I enter a string such as "Edward" or a negative number into the program, bad things happen. Users can do funny things and you want your program to be robust. One way to do that is to read in a std::string and then parse it, looking for errors.

Omit return 0

When a C or C++ program reaches the end of main the compiler will automatically generate code to return 0, so there is no need to put return 0; explicitly at the end of main.

Note: when I make this suggestion, it's almost invariably followed by one of two kinds of comments: "I didn't know that." or "That's bad advice!" My rationale is that it's safe and useful to rely on compiler behavior explicitly supported by the standard. For C, since C99; see ISO/IEC 9899:1999 section 5.1.2.2.3:

[...] a return from the initial call to the main function is equivalent to calling the exit function with the value returned by the main function as its argument; reaching the } that terminates the main function returns a value of 0.

For C++, since the first standard in 1998; see ISO/IEC 14882:1998 section 3.6.1:

If control reaches the end of main without encountering a return statement, the effect is that of executing return 0;

All versions of both standards since then (C99 and C++98) have maintained the same idea. We rely on automatically generated member functions in C++, and few people write explicit return; statements at the end of a void function. Reasons against omitting seem to boil down to "it looks weird". If, like me, you're curious about the rationale for the change to the C standard read this question. Also note that in the early 1990s this was considered "sloppy practice" because it was undefined behavior (although widely supported) at the time.

So I advocate omitting it; others disagree (often vehemently!) In any case, if you encounter code that omits it, you'll know that it's explicitly supported by the standard and you'll know what it means.

• kyrill: He does include <iostream>.
• Edward: Yes, you're right. It was <cstdlib> that was missing. Answer fixed, thanks!
• Edward: It probably is included in other headers, but it's unwise to rely on that always being true. It's an implementation detail not guaranteed by the standard.
• David: You have a lot of great points here. How about adding advice for keeping functions shorter to make it easier to read and maintain?
• Dair: Every time I see a post about C++ I think Edward is going to post something about return 0. Do you have the return 0 wall of text saved somewhere on your computer?

Answer 2

The biggest issue with your code is, as you remarked, the use of global variables. Cosmetically, the code is quite readable, but there's still room for improvement:

• Try to keep line length under 100 characters. For some people even 80 is too much.

• Variable declarations are more readable when there is one variable per line:

  int m = 0,
   n = 0,
   a[22];
  

  or even

  int m = 0;
  int n = 0;
  int a[22];
  

• It's less error-prone to always put braces after for, while, etc.

  for (int b = 0; b < 22; b++) {
   a[b] = 0;
  }
  

  If you then want to add another statement, it won't happen to you that you forget to add them and get unexpected behavior, like in this example:

  if (k == p)
   a[p]++;
   printf("incremented a[%d]\n", p);
  

• Look at this:

  if (i < chance)
   return true; //HEADS
  else
   return false; //TAILS
  

  You want to return whether i < chance. So you can do just that:

  return (i < chance);
  

  The parentheses aren't necessary, but it might be confusing without them – you be the judge.

• The runExperiment function could be split into two logical units – the experiment and printout of results. If you're motivated, you can design a data structure to contain the results. Then split the runExperiment function into two functions. The first one will run the experiment and return results, and the second one will print the results.

• While valid, it's not customary to combine C-style inlcudes with C++-style includes of C libraries:

  #include <time.h>
  #include <cmath>
  

  Preferably use only C++-style includes:

  #include <ctime>
  #include <cmath>
  

• In this small application it might be feasible to use an entire namespace, but to avoid name clashes, you ought to go through the trouble of typing std::cin, std::endl, etc. instead of just using namespace std. If you're not convinced, here's a stackoverflow question which shows how easily you can run into trouble with that.

Answer 3

All the comments from the others apply here are some more:

1. C++ has a random library you should use that to create your random number generator, as it is much mroe descriptive.

2. It is really confusing, that generate returns a boolean. For improved readability you might want to create an enumeration with values HEADS and TAIL and use that in the code.

3. Naming: You have a function that is called runExperiment. However, despite its name this does not only run "one" experiment but all of them. In principle you should try to keep your functions clean and simple, so one that generates a single experiment and one that runs over all the different experiments.

4. Classes, Classes, Classes. A lot of the input is required for every experiment run. So encapsulate this in an appropriate class maybe CoinTossExperiment or whatever.

5. Also more funtions do not hurt. Especially regarding output etc. This becomes much easier when you have encapsulated everything into a class

Answer 4

Suggestion for classes.

Why not have a class called Coin. It can have a method called Flip that returns an enum indicating Heads or Tail. Maybe some stats too (count of flips, count of heads hit,...)

Have a class called Flipper. The constructor takes an argument saying how many coins you want. It has a method called FlipAll that returns a vector of enum of Head / Tail as returned by its set of Coins.

Extra credit - flip the coins in parallel, learn how to use threads :-)

• innisfree: is OO really needed here?
• pm100: i think this is an exercise in learning c++, not a shipping library. So adding objects to it seems like a natural learning opportunity