Return to Answer

• 1

• 18.5k
• 1
• 40
• 92

There's a lot that I could say, but I'm going to limit to what I consider the most major issues:

Questionable Coding Practices

The big two are using namespace std is bad practice and rand() is considered harmful. The former is straightforward: just drop it and prefix all the std references with std::. It's only five characters and adds a great deal to readability. There's really no reason to avoid it.

For the latter, rand() just is not the best way to generate random numbers. Additionally, it is very unclear to me what is actually going in in randfloat(). It'd be much better to use the C++11 <random> library for this, specifically std::uniform_real_distribution:

float randfloat() {
 static std::random_device rd;
 static std::mt19937 gen(rd());
 static std::uniform_real_distribution<float> dis(YOUR_MIN, YOUR_MAX);
 return dis(gen);
}

I cannot tell what your actual min and max are - so fill those spaces in accordingly. Once you get over the mt19937 boilerplate, this is a much more straightforward way to do this.

Additionally, avoid macros:

#define POPSIZE 100

should be:

static constexpr int POPSIZE = 100;

And please use consistent indentation!

Weights

First, you could generate them all using std::generate instead of repeating the same line of code:

std::generate(std::begin(weights), std::end(weights), randfloat);

Secondly, prefer std::array to a raw array. This will let you do:

void TransferGenes(Perceptron A, Perceptron &B)
{
 B.weights = A.weights;
}

No loop necessary. Although here, the first argument should definitely be a reference to const to avoid the copy. And A and B are rather unhelpful names, prefer something like:

void TransferGenes(Perceptron const& src, Perceptron& dst)
{
 dst.weights = src.weights;
}

Typically the dst argument goes first (e.g. memcpy), but YMMV.

You may also want to strongly consider using double instead of float.

Passing Arrays

Writing something like:

int ChooseFittestSpecies(Perceptron pop[POPSIZE]);

is meaningless since the POPSIZE value doesn't matter. This is a function that takes a Perceptron* as an argument. So you don't actually know what the size of the array is, despite the code giving the appearance that you do. Here, I would encourage you strongly to use std::vector:

int ChooseFittestSpecies(std::vector<Perceptron>& pop);

Vectors are much easier to deal with. Plus, if somebody passes you a vector whose size is different from POPSIZE, your code will do the right thing still.

Furthermore, this particular function could take advantage of std::min_element, which clarifies the intent better:

int ChooseFittestSpecies(const std::vector<Perceptron>& pop)
{
 auto it = std::min_element(pop.begin(), pop.end(), 
 [](const Perceptron& lhs, const Perceptron& rhs){
 return lhs.error < rhs.error;
 });
 return std::distance(pop.begin(), it);
}