Your Web News in One Place

Help Webnuz

Referal links:

Sign up for GreenGeeks web hosting
November 23, 2019 03:15 pm GMT

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

Share this article:    Share on Facebook
View Full Article

Dev To

An online community for sharing and discovering great ideas, having debates, and making friends

More About this Source Visit Dev To