An Interest In:
Web News this Week
- March 31, 2024
- March 30, 2024
- March 29, 2024
- March 28, 2024
- March 27, 2024
- March 26, 2024
- March 25, 2024
Go Happy Path: the Unindented Line of Sight
While perusing how other Kubernetes developers are implementing their own reconciliation loop, I came across an interesting piece of code.
The author decided to use the if-else
control flow at its maximum potential: the logic goes as deep as three tabs to the right. We cannot immediately guess which parts are important and which aren't.
func (r *ReconcileTrial) reconcileJob(instance *trialsv1alpha3.Trial, desiredJob *unstructured.Unstructured) (*unstructured.Unstructured, error) { var err error logger := log.WithValues("Trial", types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()}) apiVersion := desiredJob.GetAPIVersion() kind := desiredJob.GetKind() gvk := schema.FromAPIVersionAndKind(apiVersion, kind) deployedJob := &unstructured.Unstructured{} deployedJob.SetGroupVersionKind(gvk) err = r.Get(context.TODO(), types.NamespacedName{Name: desiredJob.GetName(), Namespace: desiredJob.GetNamespace()}, deployedJob) if err != nil { if errors.IsNotFound(err) { if instance.IsCompleted() { return nil, nil } logger.Info("Creating Job", "kind", kind, "name", desiredJob.GetName()) err = r.Create(context.TODO(), desiredJob) if err != nil { logger.Error(err, "Create job error") return nil, err } eventMsg := fmt.Sprintf("Job %s has been created", desiredJob.GetName()) r.recorder.Eventf(instance, corev1.EventTypeNormal, JobCreatedReason, eventMsg) msg := "Trial is running" instance.MarkTrialStatusRunning(TrialRunningReason, msg) } else { logger.Error(err, "Trial Get error") return nil, err } } else { if instance.IsCompleted() && !instance.Spec.RetainRun { if err = r.Delete(context.TODO(), desiredJob, client.PropagationPolicy(metav1.DeletePropagationForeground)); err != nil { logger.Error(err, "Delete job error") return nil, err } else { eventMsg := fmt.Sprintf("Job %s has been deleted", desiredJob.GetName()) r.recorder.Eventf(instance, corev1.EventTypeNormal, JobDeletedReason, eventMsg) return nil, nil } } } return deployedJob, nil}
The outline of this function doesn't tell us anything about what the flow is and where the important logic is. Having deeply nested if-else
statements hurt Go's glanceability.
By refactoring and removing else
statements, we obtain a more coherent aligned-to-the-left path:
The green line represents the "core logic" and is at the minimum indentation level. The red line represents anything out of the ordinary: error handling and guards.
And since our eyes are very good at following lines, the line of sight (the green line) guides us and greatly improves the experience of glancing at a piece of code.
Here is the actual code I rewrote:
func (r *ReconcileTrial) reconcileJob(instance *trialsv1alpha3.Trial, desiredJob *unstructured.Unstructured) (*unstructured.Unstructured, error) { var err error logger := log.WithValues("Trial", types.NamespacedName{Name: instance.GetName(), Namespace: instance.GetNamespace()}) apiVersion := desiredJob.GetAPIVersion() kind := desiredJob.GetKind() gvk := schema.FromAPIVersionAndKind(apiVersion, kind) deployedJob := &unstructured.Unstructured{} deployedJob.SetGroupVersionKind(gvk) err = r.Get(context.TODO(), types.NamespacedName{Name: desiredJob.GetName(), Namespace: desiredJob.GetNamespace()}, deployedJob) if errors.IsNotFound(err) && instance.IsCompleted() { // Job deleted and trial completed, nothing left to do. return nil, nil } if errors.IsNotFound(err) { // Job deleted, we must create a job. logger.Info("Creating Job", "kind", kind, "name", desiredJob.GetName()) err = r.Create(context.TODO(), desiredJob) if err != nil { logger.Error(err, "Create job error") return nil, err } eventMsg := fmt.Sprintf("Job %s has been created", desiredJob.GetName()) r.recorder.Eventf(instance, corev1.EventTypeNormal, JobCreatedReason, eventMsg) msg := "Trial is running" instance.MarkTrialStatusRunning(TrialRunningReason, msg) return nil, nil } if err != nil { logger.Error(err, "Trial Get error") return nil, err } if !instance.IsCompleted() || instance.Spec.RetainRun { eventMsg := fmt.Sprintf("Job %s has been deleted", desiredJob.GetName()) r.recorder.Eventf(instance, corev1.EventTypeNormal, JobDeletedReason, eventMsg) return nil, nil } err = r.Delete(context.TODO(), desiredJob, client.PropagationPolicy(metav1.DeletePropagationForeground)) if err != nil { logger.Error(err, "Delete job error") return nil, err } return deployedJob, nil}
You can also take a look at Matt Ryer's Idiomatic Go Tricks where he presents some ways of keeping your code as readable as possible:
Original Link: https://dev.to/maelvls/go-happy-path-the-unindented-line-of-sight-25nc
Dev To
An online community for sharing and discovering great ideas, having debates, and making friendsMore About this Source Visit Dev To