Style
Species Parents[popSize];
Species Children[popSize];
It's more common for variables to start with lower case letters.
Species parents[popSize];
Species children[popSize];
That way you can easily tell that Species
is a type while parents
is a variable. As it is, I have no way to tell why Parents
is capitalized and parentA
, etc. are not.
for(int gen = 0 ; gen < 100 ; gen++){
for(int i = 0; i < popSize ; i ++)
{
You're mixing two styles here. Both are valid, but it's generally preferred to pick one and use it consistently throughout your code. More of the code is using {
on a separate line, so it may be best to standardize on that. Personally, I prefer it the other way, but as I said, both are valid.
for (int gen = 0; gen < 100; ++gen)
{
for (int i = 0; i < popSize; ++i)
{
Also, you indent inside the inner loops but not the outer. It's unclear if this was intentional or a copy/paste error. Note that if you indented the outer lines extra to make them appear in a code block, that it's often better to select the whole code and use Ctrl+K to indent them. That will increase the indent of the entire code, not just the outermost lines.
I also changed the spacing to a more common form.
Preincrement may be defined more efficiently than postincrement. It's a tiny difference, but some people prefer always using preincrement when either would work functionally. Note that a compiler may compile out the difference entirely anyway.
<
versus !=
A comment argued that it is more common to use !=
rather than <
, but I think that they missed the point. C++ favors using iterators in this situation. Since many iterators don't have a meaningful <
operator, they use something that looks sort of like
for (auto i = iterator.begin(); i != iterator.end(); iterator++)
However, that doesn't apply if your for
loop is over an int
. Because int
values do have meaningful <
, these are more robust. There's no need to maintain consistency at the price of robustness. Consider
for (int i = 0; i != 5; i +=2)
This code will crash with an integer overflow (or loop forever) because i
will never equal 5
. And this code might otherwise be a sensible replacement for
for (int i = 0; i < 5; ++i)
{
if (i % 2 == 0)
Note that if you were using std::array
or std::vector
and C++11, you could use the new range-based for
loop.
for (auto child : Children)
Then you don't have to manually manage your iteration at all.
Missing code
You use Parents[i]
but you never set it to anything. Perhaps chooseParent
or crossOver
does that for you, but it's not clear. It's also not clear why you transfereGenes
(should that be transfer rather than transfere?) with Children[i]
and Parents[i]
. Why are those two related now when they weren't earlier?
This is probably also a violation of the Single Responsibility Principle. Initialization of the Parents
and Children
arrays presumably occurs somewhere. But since it doesn't happen here or in a method called something like initialization
, it's not obvious where. Nor is it obviously correct. We're essentially relying on the function implementations being correct. But I don't even know what they are supposed to do. I can guess a bit from the names, but this code won't work at all unless one of them handles initialization.
Is your code correct? Without knowing what the code it calls does, I have no way of knowing. If you want more of a review, you may want to post a new question with more context. Preferably with runnable code.
crossOver(Parents[parentA] , Parent[ParentB] , Children[i]);
Is this a typo? Should it be
crossOver(Parents[parentA], Parents[ParentB], Children[i]);
I can't be sure. Maybe the missing code has a Parent
array as well. And of course if not, a runnable version would find that trivially with a compile.
crossOver
andtransfereGenes
\$\endgroup\$