Population.BestChromosomeChanged fires every generation with the same chromosome

[Darcara]

Describe the bug
IChromosome is being compared with != in Population.EndCurrentGeneration.
I haven't checked other places yet.

To Reproduce
Any Population that has multiple chromosomes with the same fitness. I assume this happens to me because I use ElitistReinsertion.

Expected behavior
The problem occurs when subscribing to Population.BestChromosomeChanged since != and == on interfaces will always default to reference equality the overridden/implemented equals and compareTo methods are never called. See: https://stackoverflow.com/a/5066300
Since the ordering in Generation.End cannot be stable a random chromosome is selected, reference equality fails and a different instance but the same chromosome is now the best in every generation. Calling Object.Equals(chromosome1, chromosome2) would solve this problem since this will safely call the overridden methods.

Slightly related: I would assume that for most cases a gene-based equality should be superior to a pure fitness based one.

Screenshots
If applicable, add screenshots to help explain your problem.

Version:
2.6.0

[giacomelli]

I wasn't able to reproduce the bug, can you provide a sample code, maybe a unit test exposing the problem?

[giacomelli]

Closed for inactivity. Feel free to reopen it if necessary.

[Darcara]

Sorry for the late reply @giacomelli but I just saw the closing notice. I checked my project and the problem still persists with version 3.1.2
I misdiagnosed the bug back then. The actual bug is that I implemented a custom IChromosome and implemented IComparable but neither .Equals nor .CompareTo are called for the BestChromosome-check in Population.cs:149 which was unexpected.

Not a proper unit test but this illustrates the problem:

[Test]
public void Compare() {
RandomizationProvider.Current = Substitute.For<IRandomization> ();
RandomizationProvider.Current.GetInt (0, 3).Returns (2);

IntegerChromosome integerFirst = new IntegerChromosome (0, 3);
IntegerChromosome integerSecond = new IntegerChromosome (0, 3);
IChromosome first = integerFirst;
IChromosome second = integerSecond;

Assert.AreEqual(integerFirst.ToInteger(), integerSecond.ToInteger());
Assert.AreEqual(integerFirst, integerSecond);
Assert.AreEqual(first, second);

// Any properly implemented IChromosome must pass this
Assert.True(first.CompareTo(second) == 0);
Assert.True(first.Equals(second));

// This assertion fails because it is equal to Object.ReferenceEquals(first, second)
// Neither the == operator nor .Equals() are called
Assert.True(first == second);
}

Since I assume that the reference comparison is unwanted in Population.cs:149, it should be changed to
if (BestChromosome == null || BestChromosome.CompareTo(CurrentGeneration.BestChromosome) != 0)
or (if we assume everyone will implement their chromosomes correctly)
if (!Object.Equals(BestChromosome, CurrentGeneration.BestChromosome))

ReSharper marks this as a warning: Possible unintended reference comparison

Side note: Same bug found in CheckersSquare.cs because == operator is not implemented.

If you want I can create a pull request for that.

[giacomelli]

Nice, good explanation.
Yes, I would like that you create a pull-request.

Thanks.