HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMajor

Simulating craps games

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
crapssimulatinggames

Problem

I want to adopt good coding habits now. the below code was written under tight constraints. We don't do assignment post mortems in class to discuss "real world" best practices -- we just get letter grades with little or no feedback. What are some examples of good questions I can ask myself to determine when code that works for an assignment would be re-written in real world applications?

(I'm also very interested in memory management, which we haven't covered in class. Any memory management tips re: the below code would be super-appreciated!)

Note: this is a finished assignment; I'm not looking for help to complete it, just help to develop good habits as early as possible.

Assignment:

  • simulate 5,500 craps games and print out the results.



  • other than main(), use only one function -- dice_roll(). All calculations are inline.



  • the assignment covers, enums, output precision and rand() and srand() functions.



  • use enum to define WIN, LOSE, and CONTINUE to give the results for each roll.



  • use const data sizes: int SIZE and int ROLLS.



Please feel free to provide any comments/questions as you see fit.

```
#include
#include
#include
#include

// this is what will hold our data during the games
struct nodeType
{
int num_rolls;
int win;
int loss;
nodeType* link;
};

// these pointers will be used to traverse and perform operations
// on the nodes as we figure out what to do with them
nodeType first, last, current, trailCurrent, newNode, temp;

int roll_dice();

int main()
{
enum gameOutcome { CONTINUE, WIN, LOSE };
gameOutcome game_result;
const int MAX_GAMES(5500);
int sum, point, roll;
first = NULL; // start of our list
last = NULL; // end of our list

srand(time(0)); // give rand() new seed

for (int i = 1; i link = NULL; // make sure it doesn't point to anything

switch (sum) // first roll test
{
case 7:
game_result = WIN;
roll = 1;
case 11:
game_result = WI

Solution

There are a number of things you want to ask yourself. The first is, "in the real world, do these requirements make sense?". In this case, the answer is an emphatic "No".


other than main(), use only one function -- dice_roll(). all calculations are inline.

This is a terrible requirement, and whoever wrote this assignment needs to be whacked with the common sense bat. It is teaching really bad practice right from the get go. It is never desirable to have a giant main function.

The second question to ask is, "Am I using C++ or am I using C with std::cout instead of printf?". Likely this is not your fault - any teacher who decides to make a student bundle all their code into the main function is doing a dubious job at best.

Let's tackle both of these. If you want to learn, I'm going to say throw away some of these requirements. The real requirement of the program is:


simulate 5,500 craps games and print out the results.

Here, the requirement itself leads to a relatively easy breakup of the program structure. We'll keep it simple, and say we want 4 functions in total:

  • A function to simulate dice rolls - roll_dice() (which already exists)



  • A function to simulate the games and store the results in some kind of data structure - let's call it run_simulation. This will take as a parameter the number of games to play.



  • A function to calculate and print statistics based on games. This will take the data structure from the previous function.



  • main, which will co-ordinate the above functions.



One of the nice things about C++ is the rich set of data structures available in the standard library. These generally perform memory management for you. It isn't listed in the requirements, but from your code, you want something that will store the number of wins/losses for a given number of rolls in sorted order.

Again, I'm not sure how much experience you have with data structures, however, the standard library has a good fit for this requirement: a std::map. A map stores key/value pairs in an efficient way, so that key lookup is fast.

Ok, so we'll start as before: defining our enum and something to store our game data - call it (unimaginatively) game_data:

#include 
#include 
#include 
#include 
#include     // <--- Additional include so we can use `std::map`

enum game_outcome { CONTINUE, WIN, LOSE };

struct game_data
{
    int win;
    int loss;

    game_data()
      : win(0),
        loss(0)
    { } 

    game_data(int w, int l)
      : win(w),
        loss(l)
    { }
};


Above, we've defined some constructors for our game_data type. Now, we'll define our map type:

typedef std::map result_list;


This says the type result_list is a map from integers to keys. Here, the int key will be the number of rolls. A nice thing about map is that it stores things in sorted order - so our sorting comes for free.

int roll_dice()
{
    return (rand() % 6) + (rand() % 6) + 2;
}

result_list run_simulation(int num_games)
{
    result_list results;
    static game_outcome outcome[] = {CONTINUE, CONTINUE, LOSE, LOSE, CONTINUE, CONTINUE, 
                                     CONTINUE, WIN, CONTINUE, CONTINUE, CONTINUE, WIN,
                                     LOSE};

    for(int i = 0; i < num_games; ++i) {

        int roll = 1;
        int sum = roll_dice();
        game_outcome out = outcome[sum];

        while (out == CONTINUE) {
            int point = sum;
            sum = roll_dice();
            ++roll;
            if(sum == point) {
                out = WIN;
            }
            else if(sum == 7) {
                out = LOSE;
            }
        }
        game_data& d = results[roll];
        (out == WIN) ? ++d.win : ++d.loss;
    }    

    return results;
}


Firstly, we've replaced the switch statement with an array lookup, which makes the code a bit more concise and arguably less error prone (it's easy to forget break at the end of switch statements). The while loop is mostly unchanged, other than some reordering. What has mostly changed is the replacement of a lot of code with the two lines:

game_data& d = results[roll];
(out == WIN) ? ++d.win : ++d.loss;


Lookup in a map can be done using [] syntax (much like arrays). If the key doesn't exist in this case, it is inserted. Thus, we lookup the data by rolls. For example, if rolls was 3, this would find the game_data class that sits in the map with the previous results for 3. Then, we simply increment the correct win or loss based on the outcome.

Now, we need to write a function that will take what is in the map and print out some information from what is in it: let's call it result_statistics.

```
void result_statistics(const result_list& results, int num_games)
{
int wins = 0;
int losses = 0;
double avg_length = 0;

for(auto const & p : results) {
std::cout << std::setw(4) << p.second.win << " games won and "

Code Snippets

#include <iostream>
#include <iomanip>
#include <cstdlib>
#include <ctime>
#include <map>    // <--- Additional include so we can use `std::map`

enum game_outcome { CONTINUE, WIN, LOSE };

struct game_data
{
    int win;
    int loss;

    game_data()
      : win(0),
        loss(0)
    { } 

    game_data(int w, int l)
      : win(w),
        loss(l)
    { }
};
typedef std::map<int, game_data> result_list;
int roll_dice()
{
    return (rand() % 6) + (rand() % 6) + 2;
}

result_list run_simulation(int num_games)
{
    result_list results;
    static game_outcome outcome[] = {CONTINUE, CONTINUE, LOSE, LOSE, CONTINUE, CONTINUE, 
                                     CONTINUE, WIN, CONTINUE, CONTINUE, CONTINUE, WIN,
                                     LOSE};

    for(int i = 0; i < num_games; ++i) {

        int roll = 1;
        int sum = roll_dice();
        game_outcome out = outcome[sum];

        while (out == CONTINUE) {
            int point = sum;
            sum = roll_dice();
            ++roll;
            if(sum == point) {
                out = WIN;
            }
            else if(sum == 7) {
                out = LOSE;
            }
        }
        game_data& d = results[roll];
        (out == WIN) ? ++d.win : ++d.loss;
    }    

    return results;
}
game_data& d = results[roll];
(out == WIN) ? ++d.win : ++d.loss;
void result_statistics(const result_list& results, int num_games)
{
    int wins = 0;
    int losses = 0;
    double avg_length = 0;

    for(auto const & p : results) {
        std::cout << std::setw(4) << p.second.win << " games won and " 
                  << std::setw(3) << p.second.loss << " games lost on roll "
                  << p.first << "\n";
        wins += p.second.win;
        losses += p.second.loss;
        avg_length += (p.second.win + p.second.loss) * p.first;
    }
    int total_games = wins + losses;
    std::cout << std::setiosflags(std::ios::fixed | std::ios::showpoint) 
              << "\nodds of winning are " << wins << " / "
              << total_games << " = " << std::setprecision(2)
              << 100.0 * wins / total_games << "%." << "\n";
    std::cout << "avg num rolls/game is " << std::setprecision(2)
              << avg_length / num_games << " rolls." << "\n";

}

Context

StackExchange Code Review Q#26395, answer score: 22

Revisions (0)

No revisions yet.