eaopt icon indicating copy to clipboard operation
eaopt copied to clipboard

NOffsprings has to higher than 0

Open tegk opened this issue 5 years ago • 3 comments

The unsigned int mod.NOffsprings can be never negative hence here is a logic bug.

if mod.NOffsprings <= 0 {
   return errors.New("NOffsprings has to higher than 0")
}

Could this not just be a int as uint normally used in Go just for binary operations?

models.go:217

// Validate ModDownToSize fields.
func (mod ModDownToSize) Validate() error {
	// Check the number of offsprings value
	if mod.NOffsprings <= 0 {
		return errors.New("NOffsprings has to higher than 0")
	}
	// Check the first selection method presence
	if mod.SelectorA == nil {
		return errNilSelector
	}
	// Check the first selection method parameters
	var errSelectorA = mod.SelectorA.Validate()
	if errSelectorA != nil {
		return errSelectorA
	}
	// Check the second selection method presence
	if mod.SelectorB == nil {
		return errNilSelector
	}
	// Check the second selection method parameters
	var errSelectorB = mod.SelectorB.Validate()
	if errSelectorB != nil {
		return errSelectorB
	}
	// Check the mutation rate in the presence of a mutator
	if mod.MutRate < 0 || mod.MutRate > 1 {
		return errInvalidMutRate
	}
	return nil
}

tegk avatar Oct 17 '19 08:10 tegk

Good catch! The check has to be removed.

MaxHalford avatar Oct 17 '19 13:10 MaxHalford

so simply removing

// Check the number of offsprings value
	if mod.NOffsprings <= 0 {
		return errors.New("NOffsprings has to higher than 0")
	}

is fine?

tegk avatar Oct 18 '19 08:10 tegk

Yes indeed! There might also be one or two unit test cases that will break, but I'm not sure.

MaxHalford avatar Oct 18 '19 09:10 MaxHalford