refactor for tautological condition non-nil not equal to nil #126
Conversation
| @@ -373,7 +373,7 @@ func doNotRequeue(err error) (ctrl.Result, error) { | |||
|
|
|||
| func (r *GatlingReconciler) createVolumesForCR(ctx context.Context, gatling *gatlingv1alpha1.Gatling, namespace string, log logr.Logger) error { | |||
| // Create Simulation Data ConfigMap if defined to create in CR | |||
| if &gatling.Spec.TestScenarioSpec.SimulationData != nil && len(gatling.Spec.TestScenarioSpec.SimulationData) > 0 { | |||
| if gatling.Spec.TestScenarioSpec.SimulationData != nil && len(gatling.Spec.TestScenarioSpec.SimulationData) > 0 { | |||
There was a problem hiding this comment.
When gatling is nil, the nil pointer access is panicked, so here is the case
if gatling ! = nil && gatling.Spec.TestScenarioSpec.SimulationData && len(gatling.Spec.TestScenarioSpec.SimulationData) > 0
is a good choice here.
The zero value of map is nil, so the nil check of SimulationData is left as it is.
There was a problem hiding this comment.
Thank you for the review!
It is indeed a very good idea.
However, I thought that using this approach might change the existing behavior if gatling is nil.
For example, if gatling is nil in the createVolumesForCR function, no processing is performed and error is returned as nil.
To prevent this, I think it would be better to check for nil at the beginning of the function and return an error at that point.
What do you think?
Here's what I have in mind to add at the beginning of the function.
if gatling == nil {
return errors.New("gatling is nil")
}
Description
Refactoring for
tautological condition non-nil != nilChecklist
Please check if applicable
Relevant issue https://github.com/st-tech/sre-dept-issues/issues/7361
Operation check
I ran the sample scenario and confirmed that no errors occurred.