1
\$\begingroup\$

For my assignment, I have to take a data set and stratify sample it into three different training sets (one with 10%, one with 30%, and 50%). Then I have to classify it using a Naive Bayesian classifier

I believe I did here, but since the code is very non-functional, I'm not sure if I'm going about it the right way.

Is it better to implement it using a vector for all the attributes, so I don't need to have 28 variable inside the naiveBayesian function?

#include <iostream> //std::cout
#include <fstream> //std::ifstream
#include <string> //std::string
#include <sstream> //std::istringstream
#include <vector> //std::vector
#include <algorithm> //std::remove
#include <ctime> //std::time

struct Person{
    int age;
    std::string workclass;
    int fnlwgt;
    std::string education;
    int educationNum;
    std::string maritalStatus;
    std::string occupation;
    std::string relationship;
    std::string race;
    std::string sex;
    int capitalGain;
    int capitalLoss;
    int hoursPerWeek;
    std::string nativeCountry;
    std::string salary;
};

std::vector<Person> testData;

//Prints data of single person
void printPerson(Person person){
    std::cout << person.age << " " << person.workclass << 
        " " << person.fnlwgt << " " << person.education << 
        " " << person.educationNum << " " << person.maritalStatus <<
        " " << person.occupation << " " << person.relationship <<
        " " << person.race << " " << person.sex << 
        " " << person.capitalGain << " " << person.capitalLoss << 
        " " << person.hoursPerWeek << " " << person.nativeCountry <<
        std::endl;
}

//Converts int to string
std::string convertInt(int x){
    std::string result;
    std::ostringstream convert;

    convert << x;
    result = convert.str();
    return result;
}

//Generates
int randNumGenerator(int max){
    int num = (rand() % max);
    //std::cout << num << std::endl;
    return num;
}

//Sets up all persons in data set with complete values
void setData(Person person, std::string line){

        line.erase(std::remove(line.begin(), line.end(), ','), line.end());

        std::stringstream s(line);
        std::string str;

        //Nominal attributes
        std::string workclass;
        std::string education;
        std::string maritalStatus;
        std::string occupation;
        std::string relationship;
        std::string race;
        std::string sex;
        std::string nativeCountry;

        //class label
        std::string salary;

        //Continuous attributes
        int age;
        std::string ageStr;


        int fnlwgt;
        std::string fnlwgtStr;

        int educationNum;
        std::string educationNumStr;

        int capitalGain;
        std::string capitalGainStr;

        int capitalLoss;
        std::string capitalLossStr;

        int hoursPerWeek;
        std::string hoursPerWeekStr;

        //Read in values into stringstream
        if(s >> age >> workclass >> fnlwgt >> education >> educationNum >>
            maritalStatus >> occupation >> relationship >> race >> sex >> capitalGain >>
            capitalLoss >> hoursPerWeek >> nativeCountry >> salary){

                //Convert ints into strings
                ageStr = convertInt(age);
                fnlwgtStr = convertInt(fnlwgt);
                educationNumStr = convertInt(educationNum);
                capitalGainStr = convertInt(capitalGain);
                capitalLossStr = convertInt(capitalLoss);
                hoursPerWeekStr = convertInt(hoursPerWeek);

                //Check if values are missing
                if(ageStr == "?" || workclass == "?" ||
                    fnlwgtStr == "?" || education == "?" || 
                    educationNumStr == "?" || maritalStatus == "?" ||
                    occupation == "?" || relationship == "?" ||
                    race== "?" || sex == "?" || 
                    capitalGainStr == "?" || capitalLossStr == "?" ||
                    hoursPerWeekStr == "?" || nativeCountry == "?"){

                }
                else{
                    person.age = age;
                    person.workclass = workclass;
                    person.fnlwgt = fnlwgt;
                    person.education = education;
                    person.educationNum = educationNum;
                    person.maritalStatus = maritalStatus;
                    person.occupation = occupation;
                    person.relationship = relationship;
                    person.race = race;
                    person.sex = sex;
                    person.capitalGain = capitalGain;
                    person.capitalLoss = capitalLoss;
                    person.hoursPerWeek = hoursPerWeek;
                    person.nativeCountry = nativeCountry;
                    person.salary = salary;
                    testData.push_back(person);
                }
        }

        //printPerson(person);
}

//Sets up strata for positive values
std::vector<Person> setPositive(std::vector<Person> data){
    for(int i = 0; i < testData.size(); i++){
        if(testData[i].salary == ">50K"){
            data.push_back(testData[i]);
        }
    }
    return data;
}

//Sets up strata for negative values
std::vector<Person> setNegative(std::vector<Person> data){
    for(int i = 0; i < testData.size(); i++){
        if(testData[i].salary == "<=50K"){
            data.push_back(testData[i]);
        }
    }
    return data;
}

std::vector<Person> sample(std::vector<Person> wholeDataSet, int percentage, std::vector<Person> &testingSet){
    int wholeDataSize = wholeDataSet.size();

    std::vector<Person> stratifiedSet;

    int limit = (wholeDataSize * percentage) / 100;
    int randNum= 0;

    std::vector<bool> numsUsedAlready(wholeDataSize);

    for(int i = 0; i < limit; i++){
        randNum = randNumGenerator(wholeDataSize);
        while(numsUsedAlready[randNum]){
            randNum = randNumGenerator(wholeDataSize);
        }
        //std::cout << "Done generating " << i << " limit: " << limit << std::endl; 
        numsUsedAlready[randNum] = true;
        stratifiedSet.push_back(wholeDataSet[randNum]); 
        //wholeDataSet.erase(wholeDataSet.begin() + randNum);
    //  std::cout << "Done erasing " << i << " limit: " << limit << std::endl;
    } 
    for(int i = 0; i < numsUsedAlready.size(); i++){
        if(!numsUsedAlready[i]){
            testingSet.push_back(wholeDataSet[i]);
        }
    }

    //delete numsUsedAlready
    return stratifiedSet;
}

std::vector<Person> concatVectors(std::vector<Person> a, std::vector<Person> b){
    std::vector<Person> ab;
    ab.reserve(a.size() + b.size());
    ab.insert(ab.end(), a.begin(), a.end()); //Add a
    ab.insert(ab.end(), b.begin(), b.end()); //Add b
    return ab;
}

void compareAttributeInt(int sample, int trained, std::string salary, int &count, int posOrNeg){
    if(sample == trained){
        if(salary == ">50K" && posOrNeg == 1){
            count++;
        }
        if(salary == "<=50K" && posOrNeg == 0){
            count++;
        }
    }
}

void compareAttributeStr(std::string sample, std::string trained, std::string salary, int &count, int posOrNeg){
    if(sample == trained){
        if(salary == ">50K" && posOrNeg == 1){
            count++;
        }       
        if(salary == "<=50K" && posOrNeg == 0){
            count++;
        }

    }
}

void setToZero(int &a, int &b, int &c, int &d, int &e, int &f, int &g, int &h, int &i,
        int &j, int &k, int &l, int &m){
    a = 0;
    b = 0;
    c = 0;
    d = 0;
    e = 0;
    f = 0;
    g = 0;
    h = 0;
    i = 0;
    j = 0;
    k = 0;
    l = 0;
    m = 0;
}

float naiveBayesian(std::vector<Person> trainingSet, std::vector<Person> testingSet){
    float accuracy = 0;
    int accuracyCount = 0;
    int randNum = 0;

    std::vector<Person> sampleSet;
    std::vector<bool> numsUsedAlready(testingSet.size());

    for(int i = 0; i < 20; i++){
        randNum = randNumGenerator(testingSet.size());
        while(numsUsedAlready[randNum]){
            randNum = randNumGenerator(testingSet.size());
        }
        numsUsedAlready[randNum] = true;
        sampleSet.push_back(testingSet[randNum]);
    }

    int posAgeCount = 0;
    int posWorkclassCount = 0;
    int posFnlwgtCount = 0;
    int posEducationCount = 0;
    int posEducationNumCount = 0;
    int posMaritalStatusCount = 0;
    int posOccupationCount = 0;
    int posRaceCount = 0;
    int posRelationshipCount = 0;
    int posSexCount = 0;
    int posCapitalGainCount = 0;
    int posCapitalLossCount = 0;
    int posHoursPerWeekCount = 0;
    int posNativeCountryCount = 0;

    int negAgeCount = 0;
        int negWorkclassCount =0;
        int negFnlwgtCount = 0;
        int negEducationCount = 0;
        int negEducationNumCount = 0;
        int negMaritalStatusCount = 0;
        int negOccupationCount = 0;
        int negRaceCount = 0;
    int negRelationshipCount = 0;
        int negSexCount = 0;
        int negCapitalGainCount = 0;
        int negCapitalLossCount = 0;
        int negHoursPerWeekCount = 0;
        int negNativeCountryCount = 0;

    float probPosAgeCount = 0;
    float probPosWorkclassCount = 0;
    float probPosFnlwgtCount = 0;
    float probPosEducationCount = 0;
    float probPosEducationNumCount = 0;
    float probPosMaritalStatusCount = 0;
    float probPosOccupationCount = 0;
    float probPosRaceCount = 0;
    float probPosRelationshipCount = 0;
    float probPosSexCount = 0;
    float probPosCapitalGainCount = 0;
    float probPosCapitalLossCount = 0;
    float probPosHoursPerWeekCount = 0;
    float probPosNativeCountryCount = 0;

    float probNegAgeCount = 0;
        float probNegWorkclassCount =0;
        float probNegFnlwgtCount = 0;
        float probNegEducationCount = 0;
        float probNegEducationNumCount = 0;
        float probNegMaritalStatusCount = 0;
        float probNegOccupationCount = 0;
        float probNegRaceCount = 0;
    float probNegRelationshipCount = 0;
        float probNegSexCount = 0;
        float probNegCapitalGainCount = 0;
        float probNegCapitalLossCount = 0;
        float probNegHoursPerWeekCount = 0;
        float probNegNativeCountryCount = 0;

    int numOver50k = 0;
    int numUnder50k = 0;

    for(int i = 0; i < trainingSet.size(); i++){
        if(trainingSet[i].salary == ">50K"){
            numOver50k++;
        }
        else{
            numUnder50k++;
        }
    }

    float probOver50k = (float)numOver50k / trainingSet.size();
    float probUnder50k = (float)numUnder50k / trainingSet.size();

    float probYes = 0;
    float probNo = 0;

    float yes= 0;
    float no = 0;

    //int salaryCount;

    bool salaryGreaterThan50k = false;
    for(int i = 0; i < sampleSet.size(); i++){
        Person sample = sampleSet[i];

        for(int j = 0; j < trainingSet.size(); j++){
            Person trained = trainingSet[j];
            if(sample.age == trained.age){
                if(trained.salary == ">50K"){
                    posAgeCount++;
                }
                else{
                    negAgeCount++;
                }
            }
            if(sample.workclass == trained.workclass){
                if(trained.salary == ">50K"){
                    posWorkclassCount++;
                }
                else{
                    negWorkclassCount++;
                }
            }
            if(sample.fnlwgt == trained.fnlwgt){
                if(trained.salary == ">50K"){
                    posFnlwgtCount++;
                }
                else{
                    negFnlwgtCount++;
                }
            }
            if(sample.education == trained.education){
                if(trained.salary == ">50K"){
                    posEducationCount++;
                }
                else{
                    negEducationCount++;
                }
            }
            if(sample.educationNum == trained.educationNum){
                if(trained.salary == ">50K"){
                    posEducationNumCount++;
                }
                else{
                    negEducationNumCount++;
                }
            }
            if(sample.maritalStatus == trained.maritalStatus){
                if(trained.salary == ">50K"){
                    posMaritalStatusCount++;
                }
                else{
                    negMaritalStatusCount++;
                }
            }
            if(sample.occupation == trained.occupation){
                if(trained.salary == ">50K"){
                    posOccupationCount++;
                }
                else{
                    negOccupationCount++;
                }
            }
            if(sample.race == trained.race){
                if(trained.salary == ">50K"){
                    posRaceCount++;
                }
                else{
                    negRaceCount++;
                }
            }
            if(sample.relationship == trained.relationship){
                if(trained.salary == ">50K"){
                    posRelationshipCount++;
                }
                else{
                    negRelationshipCount++;
                }
            }
            if(sample.sex == trained.sex){
                if(trained.salary == ">50K"){
                    posSexCount++;
                }
                else{
                    negSexCount++;
                }
            }
            if(sample.capitalGain == trained.capitalGain){
                if(trained.salary == ">50K"){
                    posCapitalGainCount++;
                }
                else{
                    negCapitalGainCount++;
                }
            }
            if(sample.capitalLoss == trained.capitalLoss){
                if(trained.salary == ">50K"){
                    posCapitalLossCount++;
                }
                else{
                    negCapitalLossCount++;
                }
            }
            if(sample.hoursPerWeek == trained.hoursPerWeek){
                if(trained.salary == ">50K"){
                    posHoursPerWeekCount++;
                }
                else{
                    negHoursPerWeekCount++;
                }
            }
            if(sample.nativeCountry == trained.nativeCountry){
                if(trained.salary == ">50K"){
                    posNativeCountryCount++;
                }
                else{
                    negNativeCountryCount++;
                }
            }

        }//end innner loop  


        //Calculate successful probabilites
        probPosAgeCount = (float)posAgeCount / numOver50k;

        probPosWorkclassCount = (float)posWorkclassCount / numOver50k;

        probPosFnlwgtCount = (float)posFnlwgtCount / numOver50k;

        probPosEducationCount = (float)posEducationCount / numOver50k;

        probPosEducationNumCount = (float)posEducationNumCount / numOver50k;

        probPosMaritalStatusCount = (float)posMaritalStatusCount / numOver50k;

        probPosOccupationCount = (float)posOccupationCount / numOver50k;

        probPosRaceCount = (float)posRaceCount / numOver50k;

        probPosRelationshipCount = (float)posRelationshipCount / numOver50k;

        probPosSexCount = (float)posSexCount / numOver50k;

        probPosCapitalGainCount = (float)posCapitalGainCount / numOver50k;

        probPosCapitalLossCount = (float)posCapitalLossCount / numOver50k;

        probPosHoursPerWeekCount = (float)posHoursPerWeekCount / numOver50k;

        probPosNativeCountryCount = (float)posNativeCountryCount / numOver50k;

        //Calculate failing probabilities
        probNegAgeCount = (float) negAgeCount / numUnder50k; 

        probNegWorkclassCount = (float) negWorkclassCount / numUnder50k;

        probNegFnlwgtCount = (float) negFnlwgtCount / numUnder50k;

        probNegEducationCount = (float) negEducationCount / numUnder50k;

        probNegEducationNumCount = (float) negEducationNumCount / numUnder50k;

        probNegMaritalStatusCount = (float) negMaritalStatusCount / numUnder50k;

        probNegOccupationCount = (float) negOccupationCount / numUnder50k;

        probNegRaceCount = (float) negRaceCount / numUnder50k;

        probNegRelationshipCount = (float) negRelationshipCount / numUnder50k;

        probNegSexCount = (float) negSexCount / numUnder50k;

        probNegCapitalGainCount =(float) negCapitalGainCount / numUnder50k;

        probNegCapitalLossCount =(float) negCapitalLossCount / numUnder50k;

        probNegHoursPerWeekCount =  (float) negHoursPerWeekCount / numUnder50k;

                probNegNativeCountryCount = (float) negNativeCountryCount / numUnder50k;

        probYes = (float)probPosAgeCount * probPosWorkclassCount * probPosFnlwgtCount *
            probPosEducationCount * probPosEducationNumCount * probPosMaritalStatusCount *
            probPosOccupationCount * probPosRaceCount * probPosRelationshipCount * 
            probPosSexCount * probPosCapitalGainCount * probPosCapitalLossCount * 
            probPosHoursPerWeekCount * probPosNativeCountryCount;

        probNo = (float)probNegAgeCount * probNegWorkclassCount * probNegFnlwgtCount *
            probNegEducationCount * probNegEducationNumCount * probNegMaritalStatusCount *
            probNegOccupationCount * probNegRaceCount * probNegRelationshipCount * 
            probNegSexCount * probNegCapitalGainCount * probNegCapitalLossCount * 
            probNegHoursPerWeekCount * probNegNativeCountryCount;

        yes = (float)probYes * probOver50k;
        no = (float)probNo * probUnder50k;

        if(yes > no){
            salaryGreaterThan50k = true;
        }
        else{
            salaryGreaterThan50k = false;
        }
        if(salaryGreaterThan50k){
            if(sample.salary == ">50K"){
                accuracyCount++;
            }
        }
        else{
            if(sample.salary == "<=50K"){
                accuracyCount++;
            }
        }

        setToZero(posAgeCount, posWorkclassCount, posFnlwgtCount,
            posEducationCount, posEducationNumCount, posMaritalStatusCount, 
            posOccupationCount, posRaceCount, posSexCount, 
            posCapitalGainCount, posCapitalLossCount, posHoursPerWeekCount, 
            posNativeCountryCount);

        setToZero(negAgeCount, negWorkclassCount, negFnlwgtCount,
            negEducationCount, negEducationNumCount, negMaritalStatusCount, 
            negOccupationCount, negRaceCount, negSexCount, 
            negCapitalGainCount, negCapitalLossCount, negHoursPerWeekCount, 
            negNativeCountryCount);
    }//end outer loop

    accuracy = (float)accuracyCount / 20;
    //std::cout << "accuracy " << accuracy << std::endl;
    return accuracy;
}

void stratifiedSample(){
    srand(time(NULL)); 

    std::vector<Person> positiveSamples;
    std::vector<Person> negativeSamples;
    positiveSamples = setPositive(positiveSamples);
    //std::cout << "done grouping positives" << std::endl;

    negativeSamples = setNegative(negativeSamples);
    //std::cout << "done grouping negatives" << std::endl;  

    std::vector<Person> posTestingSet10 = positiveSamples;
    std::vector<Person> posTestingSet30 = positiveSamples;
    std::vector<Person> posTestingSet50 = positiveSamples;

    std::vector<Person> negTestingSet10 = negativeSamples;
    std::vector<Person> negTestingSet30 = negativeSamples;
    std::vector<Person> negTestingSet50 = negativeSamples;

    std::vector<Person> posStratifiedSet_10;
    std::vector<Person> posTesting_10;
    std::vector<Person> negStratifiedSet_10;
    std::vector<Person> negTesting_10;

    std::vector<Person> posStratifiedSet_30;
    std::vector<Person> posTesting_30;
    std::vector<Person> negStratifiedSet_30;
    std::vector<Person> negTesting_30;

    std::vector<Person> posStratifiedSet_50;
    std::vector<Person> posTesting_50;
    std::vector<Person> negStratifiedSet_50;
    std::vector<Person> negTesting_50;

    std::vector<Person> stratifiedSet_10;
    std::vector<Person> stratifiedSet_30;
    std::vector<Person> stratifiedSet_50;

    std::vector<Person> testingSet_10;
    std::vector<Person> testingSet_30;
    std::vector<Person> testingSet_50;

    posStratifiedSet_10 = sample(posTestingSet10, 10, posTesting_10);
    //std::cout << "Done w/ positive10" << std::endl;

    negStratifiedSet_10 = sample(negTestingSet10, 10, negTesting_10);
    //std::cout << "Done w/ negative10" << std::endl;

    posStratifiedSet_30 = sample(posTestingSet30, 30, posTesting_30);
    //std::cout << "Done w/ positive30" << std::endl;
    negStratifiedSet_30 = sample(negTestingSet30, 30, negTesting_30);
    //std::cout << "Done w/ negative30" << std::endl;

    posStratifiedSet_50 = sample(posTestingSet50, 50, posTesting_50);
//  std::cout << "Done w/ positive50" << std::endl;
    negStratifiedSet_50 = sample(negTestingSet50, 50, negTesting_50);
//  std::cout << "Done w/ negative50" << std::endl;

    stratifiedSet_10 = concatVectors(posStratifiedSet_10, negStratifiedSet_10);
    stratifiedSet_30 = concatVectors(posStratifiedSet_30, negStratifiedSet_30);
    stratifiedSet_50 = concatVectors(posStratifiedSet_50, negStratifiedSet_50);

    testingSet_10 = concatVectors(posTesting_10, negTesting_10);
    testingSet_30 = concatVectors(posTesting_30, negTesting_30);
    testingSet_50 = concatVectors(posTesting_50, negTesting_50);

    //std::cout << "size10: " << stratifiedSet_10.size() << " testingSet10: " << testingSet_10.size() << std::endl;
    //std::cout << "size30: " << stratifiedSet_30.size() << " testingSet30: " << testingSet_30.size() << std::endl;
    //std::cout << "size50: " << stratifiedSet_50.size() << " testingSet50: " << testingSet_50.size() << std::endl;

    float accuracy10 = 0;
    float accuracy30 = 0;
    float accuracy50 = 0;

    accuracy10 = naiveBayesian(stratifiedSet_10, testingSet_10);
    std::cout << "accuracy for 10%: " << accuracy10 << std::endl;

    accuracy30 = naiveBayesian(stratifiedSet_30, testingSet_30);
    std::cout << "accuracy for 30%: " << accuracy30 << std::endl;

    accuracy50 = naiveBayesian(stratifiedSet_50, testingSet_50);
    std::cout << "accuracy for 50%: " << accuracy50 << std::endl;

}

//Reads the file
void readInputFile(std::ifstream &file){
    std::string line;
    while(getline(file,line)){
        Person person;
        setData(person, line);
    }
}

//Prints the usage string
void usageString(){
    std::cout << "Usage: myProgram <input_file>" << std::endl;
}


int main(int argc, char** argv){
    const char *inputfile;

    if (argc < 2){
        usageString();
        return EXIT_FAILURE;
    }
    else{
        inputfile = argv[1];
    }

    std::ifstream input(inputfile);
    if(!input.is_open()){
        std::cerr << "Error: Data file doesn't exist" << std::endl;
        return EXIT_FAILURE;
    }

    readInputFile(input);
    //std::cout << "done reading" << std::endl;
    stratifiedSample();
    return 1;
}
\$\endgroup\$
4
\$\begingroup\$

Lots and lots of copies

All of your functions are passing data by value. You're incurring lots and lots of copies completely unnecessarily. If you're not modifying your input, you should take it by reference to const. If you don't need to make a copy of something, take a reference to it.

Follow Convention

The C++ convention for printing an object is to override std::ostream& operator<<(std::ostream&, const Person&) and the convention for reading an object for a stream is to override std::istream& operator>>(std::istream&, Person& ). This would let you print your person like:

std::cout << person << std::endl;

Avoid Globals

You have testData as a global. This makes all the functions that access it decidedly non-functional. setPositive and setNegative confused me for a while, as they take a vector<Person> as input but don't actually use it as input. This would make more sense:

std::vector<Person> setPositive(const std::vector<Person>& testData)
{
    std::vector<Person> data;
    for (int i = 0; i < testData.size(); ++i) {
        if (testData[i].salary == ">50K") {
            data.push_back(testData[i]);
        }
    }
    return data;
}

Code Duplication

This is the big one. Let's start small and work our way up.

setPositive and setNegative do basically the same thing, so it makes sense to factor that out. They're both filters, so let's write them as such:

template <typename Predicate>
std::vector<Person> filter(const std::vector<Person>& testData, Predicate pred) {
    std::vector<Person> out;
    std::copy_if(testData.begin(), testData.end(), std::back_inserter(out), pred);
    return out;
}

Now you can have:

std::vector<Person> positive = filter(testData, [](const Person& p){ return p.salary == ">50K");

That's very functional.

Now, let's go in your naiveBayesian() function. Your overall structure is:

for (const Person& sample : sampleSet)
{
    for (const Person& trained : trainingSet) 
    {
        // lots of stuff that looks like...
        if (sample.X == trained.X) {
            if (trained.salary == ">50") {
                posX++;
            }
            else {
                negX++;
            }
        }
    }

    // zero everything
    posX = 0;
    negX = 0;
}

So as a first go, we can drop the need for setToZero() by simply declaring all the pos/neg variables in the outer loop:

for (const Person& sample : sampleSet)
{
    // for lots of X...
    int posX = 0, negX = 0;

    for (const Person& trained : trainingSet) 
    {
        // lots of stuff that looks like...
        if (sample.X == trained.X) {
            if (trained.salary == ">50") {
                posX++;
            }
            else {
                negX++;
            }
        }
    }
}

You always have these positive/negative counts together. So let's keep them together:

struct Count {
    int pos = 0;
    int neg = 0;
};

We can even add a function that takes a flag to do the increment:

struct Count {
    void increment(bool flag) {
        if (flag) {
            ++pos;
        }
        else {
            ++neg;
        }
    }
};

That reduces the above to:

for (const Person& sample : sampleSet)
{
    Count age, workClass, Fnlwgt, ...;

    for (const Person& trained : trainingSet) 
    {
        if (sample.age == trained.age) {
            age.increment(trained.salary == ">50K");
        }

        if (sample.workClass == trained.workClass) {
            workClass.increment(trained.salary == ">50K");
        }

        ...
    }
}

It's better, but still verbose. I would say... just flip the loop. Instead of looping over the training set one time, and then checking N different things... loop over the set N times, checking one different thing each. That is:

for (const Person& sample : sampleSet)
{
    Count age = compute_count(sample, trainingSet, &Person::age);
    Count workClass = compute_count(sample, trainingSet, &Person::workclass);
    Count fnlwgt = compute_count(sample, trainingSet, &Person::fnlwgt);
    ...
}

with:

template <typename T>
Count compute_count(const Person& sample, const std::vector<Person>& trainingSet, T Person::*member)
{
    Count res;
    for (const Person& training : trainingSet) {
        if ((sample.*member) == (training.*member)) {
            res.increment(training.salary == ">50K");
        }
    }
    return res;
}

That is much shorter, and no longer repetitive.

Declare Variables where you use them

This is a very C-style list of variable declarations. Instead of putting them all up top, just define them inline. And prefer double to float:

double probYes = 1.0 * (age.pos / numOver50k)
                     * (workClass.pos / numOver50k)
                     * ...

double probNo = ...;

bool salaryGreaterThan50k = (probYes * probOver50k) > (probNo * probUnder50k);

// we want A && B or !A && !B. That's the same as !(A ^ B)
if (!(salaryGreaterThan50k ^ (sample.salary == ">50K")) {
    ++accuracyCount;
}

And then this can be further reduced! Since we never care about each Count individually, let's make a vector of them:

std::vector<Count> counts;
counts.push_back(compute_count(sample, trainingSet, &Person::age));
counts.push_back(compute_count(sample, trainingSet, &Person::workclass));
counts.push_back(compute_count(sample, trainingSet, &Person::fnlwgt));
...

Now, the probs become:

double probYes = std::accumulate(counts.begin(), counts.end(),
                                 1.0, // initial value
                                 [=](double init, const Count& count){
                                     return init * count.pos / numOver50k;
                                 });

double probNo  = std::accumulate(counts.begin(), counts.end(),
                                 1.0, // initial value
                                 [=](double init, const Count& count){
                                     return init * count.neg / numUnder50k;
                                 });

Now, if you want to add a new attribute, you need to add one line of code. Just another push_back. That's tough to beat!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.