1
\$\begingroup\$

I am writing a handler to render a GIF of a Lorentz attractor. I need to parse floating point numbers from the some querystrings attached to the GIF path. If any of them are bad, I need to log an error.

func lorenzHandler(w http.ResponseWriter, r *http.Request) {
    var e error
    rho := atof(r.FormValue("r"), &e)
    sigma := atof(r.FormValue("s"), &e)
    b := atof(r.FormValue("b"), &e)
    if e != nil {
        log.Printf("bad parameters: r=%s; s=%s; b=%s;\n", r.FormValue("r"), r.FormValue("s"), r.FormValue("b"))
    } else {
        // draw lorenz GIF with parameters rho, sigma and b
        // but until then...
        log.Println("yay!", rho, sigma, b)
    }
}


func atof(s string, e *error) float32 {
    if *e == nil {
        var x float64
        x, *e = strconv.ParseFloat(s, 32)
        return float32(x)
    }
    return 0
}

Is passing around an error, then cleaning it up, correct?

\$\endgroup\$

1 Answer 1

Reset to default
3
\$\begingroup\$

I miss the reason you are working with a pointer of an error and passing it in different functions.

Why not doing in the simple way?

func lorenzHandler(w http.ResponseWriter, r *http.Request) {
    rho, e1 := strconv.ParseFloat(r.FormValue("r"), 32)
    sigma, e2 := strconv.ParseFloat(r.FormValue("s"), 32)
    b, e3 := strconv.ParseFloat(r.FormValue("b"), 32)
    if e1 != nil || e2 != nil || e3 != nil {
        log.Printf("bad parameters: r=%s; s=%s; b=%s;\n", r.FormValue("r"), r.FormValue("s"), r.FormValue("b"))
        w.WriteHeader(http.StatusBadRequest)
        return
    }

    // draw lorenz GIF with parameters rho, sigma and b
    // but until then...
    log.Println("yay!", rho, sigma, b)
}

If the problem is about performance, and you want to stop after the first error, without waiting all three executions, you can do

func lorenzHandler(w http.ResponseWriter, r *http.Request) {
    rho, err := strconv.ParseFloat(r.FormValue("r"), 32)
    if err != nil {
        log.Printf("bad parameters: r=%s\n", r.FormValue("r"))
        w.WriteHeader(http.StatusBadRequest)
        return
    }

    sigma, err := strconv.ParseFloat(r.FormValue("s"), 32)
    if err != nil {
        log.Printf("bad parameters: s=%s\n", r.FormValue("s"))
        w.WriteHeader(http.StatusBadRequest)
        return
    }

    b, err := strconv.ParseFloat(r.FormValue("b"), 32)
    if err != nil {
        log.Printf("bad parameters: b=%s\n", r.FormValue("b"))
        w.WriteHeader(http.StatusBadRequest)
        return
    }

    // draw lorenz GIF with parameters rho, sigma and b
    // but until then...
    log.Println("yay!", rho, sigma, b)
}
\$\endgroup\$
3
  • \$\begingroup\$ I heard the idea, but had not seen the doc, that errors are values. I was trying to see if this was an application. After looking at your solution and the doc I am not quite sure what the right approach is. I am not saying I think your approach goes against what the document is advising. Just the combination of your answer and the document leaves me confused. \$\endgroup\$
    – yberman
    Commented Aug 8, 2017 at 5:25
  • 1
    \$\begingroup\$ What leaves you confused? Sure, errors are values, and for the language you can do what you did, but using the same variable for three different calls, passing it with a pointer is a lack of readability and is error-prone (in a very simple functions like this). I think the most idiomatic way to do it in Go is my second example. * "Return early" handling all bad cases and then work with standard case in a non-nested body * I understand it's a bit verbose, so I prefer my first example (when I can merge some error handling together). \$\endgroup\$
    – LucianoQ
    Commented Aug 8, 2017 at 8:21
  • 1
    \$\begingroup\$ Thanks. I usually use C where f(..., &r) is not confusing for return. But here when multiple return values are available, persisting a single tiny object makes no sense. \$\endgroup\$
    – yberman
    Commented Aug 8, 2017 at 14:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.