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

Fun with probability theory

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

Problem

I want to play around with the Powerball lotto drawing history. What other probability theories could I try or what other probability class libraries exist. The code parses the file very quickly but is there a better way to go about it?

I am working on a multiple regression analysis with the first white_ball as a dependent variable.

```
#include
#include
#include
#include
#include "linear.h" // http://www.codecogs.com/code/maths/approximation/regression/linear.php
#include

using namespace std;

// Powerball rules.
// (http://en.wikipedia.org/wiki/Powerball)
const int POWERBALL_WHITE = 59; // White balls number 1-59
const int WHITE_DRAWS = 5; // Only 5 balls are drawn.
const int POWERBALL_RED = 35; // Red balls number 1-35
const int REB_DRAWS = 1; // Only 1 ball is drawn.

// Parse rules.
const int MAX_LINE = 37;
const char* DELIMITER_DATE = "/"; // Draw date format is separated by forward slash.

typedef struct {
int month;
int day;
int year;
int white_ball_1;
int white_ball_2;
int white_ball_3;
int white_ball_4;
int white_ball_5;
int reb_ball;
} Powerball;

int main (int argc, char** argv) {
// Open the historical data file.
// (http://www.powerball.com/powerball/pb_nbr_history.asp)
ifstream lotto;
lotto.open("lotto.txt");
if (!lotto.good())
return 1;

// How many weekly drawing have taken place?
int number_Drawings = (int)count(istreambuf_iterator(lotto), istreambuf_iterator(), '\n') - 1;
lotto.seekg(0);
// Create Powerball drawing array.
Powerball *drawings;
drawings = (Powerball) malloc(number_Drawings sizeof(Powerball));

// Parse loto results to Powerball array.
lotto.ignore(50, '\n'); // Skip first line with category references.
while (!lotto.eof()) {
Powerball draw;

// Line objects to parse.
static int n;
int month, day, year;
char DELIMITER; // Date delimiter like variable

Solution

My suggestions won't make your code faster, I focused on idiomatic and clear code. It's already quite nice as it is. :)

// Powerball rules.
// (http://en.wikipedia.org/wiki/Powerball)
const int POWERBALL_WHITE = 59;     // White balls number 1-59
const int WHITE_DRAWS = 5;          // Only 5 balls are drawn.
const int POWERBALL_RED = 35;       // Red balls number 1-35
const int REB_DRAWS = 1;            // Only 1 ball is drawn.


Did you mean RED_DRAWS? You're not using any of those constants. Interestingly, it does make the code clearer for someone who doesn't know the rules.

// Parse rules.
const int MAX_LINE = 37;
const char* DELIMITER_DATE = "/";   // Draw date format is separated by forward slash.

typedef struct {
    int month;
    int day;
    int year;
    int white_ball_1;
    int white_ball_2;
    int white_ball_3;
    int white_ball_4;
    int white_ball_5;


A static array would be easier here: int whiteballes[WHITE_DRAWS].

int reb_ball;


Did you mean red_ball?

} Powerball;


This is a C-style struct, but a C++-style struct would have been more idiomatic.

int main (int argc, char** argv) {
    // Open the historical data file.
    // (http://www.powerball.com/powerball/pb_nbr_history.asp)
    ifstream lotto;
    lotto.open("lotto.txt");


Isn't it simpler to use the constructor of std::ifstream?

if (!lotto.good())
        return 1;


This can be very surprising if nothing is printed on std::cerr.

// How many weekly drawing have taken place?
    int number_Drawings = (int)count(istreambuf_iterator(lotto), istreambuf_iterator(), '\n') - 1;
    lotto.seekg(0);
    // Create Powerball drawing array.
    Powerball *drawings;
    drawings = (Powerball*) malloc(number_Drawings * sizeof(Powerball));


C-style allocation, when you could have used a C++03 vector or a C++11 fixed-size array. You're not even freeing this memory.

// Parse loto results to Powerball array.
    lotto.ignore(50, '\n'); // Skip first line with category references.


Avoid magic numbers, this a good fit for a constant.

while (!lotto.eof()) {
        Powerball draw;

        // Line objects to parse.
        static int n;
        int month, day, year;
        char DELIMITER; // Date delimiter like variable.
        int white_1,white_2, white_3, white_4, white_5, red;

        lotto >> month >> DELIMITER >> day >> DELIMITER >> year >> white_1 >> white_2 >> white_3 >> white_4 >> white_5 >> red;


Why don't you read directly in draw? This would make the following lines unnecessary.

draw.month = month;
        draw.day = day;
        draw.year = year;
        draw.white_ball_1 = white_1;
        draw.white_ball_2 = white_2;
        draw.white_ball_3 = white_3;
        draw.white_ball_4 = white_4;
        draw.white_ball_5 = white_5;
        draw.reb_ball = red;

        drawings[n++] = draw;
        if (lotto.peek() != '\n') // For entries with Plus Ball or extra characters
            lotto.ignore(10, '\n');


General formatting advice: avoid long lines and spaces after instructions.

}
    lotto.close();

    // Create linear regression arrays variables.
    double lin_reg_1[number_Drawings];
    double lin_reg_2[number_Drawings];
    double lin_reg_3[number_Drawings];
    double lin_reg_4[number_Drawings];
    double lin_reg_5[number_Drawings];


What about a matrix?

for (int i = 0; i < number_Drawings; i++) {
        lin_reg_1[i] = drawings[i].white_ball_1;
        lin_reg_2[i] = drawings[i].white_ball_2;
        lin_reg_3[i] = drawings[i].white_ball_3;
        lin_reg_4[i] = drawings[i].white_ball_4;
        lin_reg_5[i] = drawings[i].white_ball_5;
    }


If white_ball was an array, you could have only one affectation in your loop.

Code Snippets

// Powerball rules.
// (http://en.wikipedia.org/wiki/Powerball)
const int POWERBALL_WHITE = 59;     // White balls number 1-59
const int WHITE_DRAWS = 5;          // Only 5 balls are drawn.
const int POWERBALL_RED = 35;       // Red balls number 1-35
const int REB_DRAWS = 1;            // Only 1 ball is drawn.
// Parse rules.
const int MAX_LINE = 37;
const char* DELIMITER_DATE = "/";   // Draw date format is separated by forward slash.

typedef struct {
    int month;
    int day;
    int year;
    int white_ball_1;
    int white_ball_2;
    int white_ball_3;
    int white_ball_4;
    int white_ball_5;
int reb_ball;
} Powerball;
int main (int argc, char** argv) {
    // Open the historical data file.
    // (http://www.powerball.com/powerball/pb_nbr_history.asp)
    ifstream lotto;
    lotto.open("lotto.txt");

Context

StackExchange Code Review Q#26130, answer score: 6

Revisions (0)

No revisions yet.