Ytdyklly

What is does

I'm reading The Go Programming Language. Below is my code for the last exercise in the first chapter, which is to create a webserver, generate an image, and serve the image via the server, allowing modifications of the image via query string parameters, eg: localhost:8000/?cycles=25&bgcolor=0,255,255.

My Concerns

I'm open to any and all suggestions, but any advice specifically concerning these points would be especially helpful.

Duplicated code. There's just enough duplication to trigger my OCD, but each one is slightly different so I'm not sure how I could delegate to a function without creating a bunch of very similar functions. In particular, the two for loops where I'm parsing colors really bother me.

Type Juggling. strconv.ParseInt has a third argument to specify the bit size but always returns int64 anyway, so I still have to explicitly cast then to unsigned 8 bit integers. Take a look at my img.SetColorIndex call where I'm doing all sorts of casting just for some simple arithmetic.

Concision. I am baffled at the fact that I can write a simple server using the standard library with about 4 lines of code, but turning a string into an array of numbers took me 19 lines of code. And I had to use basically the same 19 lines twice because I couldn't figure out what to specify as the type when passing a color.RGBA as a function parameter.

package main



import (

    "log"

    "net/http"

    "io"



    "image"

    "image/color"

    "image/gif"

    "math"

    "math/rand"



    "strconv"

    "strings"

)



var bgcolor = color.RGBA{0, 0, 0, 255}

var fgcolor = color.RGBA{255, 255, 255, 255}



func main() {

    http.HandleFunc("/", serveImage)

    log.Fatal(http.ListenAndServe("localhost:8000", nil))

}



func serveImage(w http.ResponseWriter, r* http.Request) {

    if err := r.ParseForm(); err != nil {

        log.Print(err)

    }

    cycles := 10.0

    res := 0.001

    size := 100

    frames := 64

    delay := 8

    if value, exists := r.Form["cycles"]; exists {

        if v, err := strconv.ParseFloat(value[0], 64); err == nil {

            cycles = v

        }

    }

    if value, exists := r.Form["res"]; exists {

        if v, err := strconv.ParseFloat(value[0], 64); err == nil {

            res = v

        }

    }

    if value, exists := r.Form["size"]; exists {

        if v, err := strconv.ParseFloat(value[0], 64); err == nil {

            size = int((v-1)/2)

        }

    }

    if value, exists := r.Form["frames"]; exists {

        if v, err := strconv.ParseInt(value[0], 10, 0); err == nil {

            frames = int(v)

        }

    }

    if value, exists := r.Form["delay"]; exists {

        if v, err := strconv.ParseInt(value[0], 10, 0); err == nil {

            delay = int(v)

        }

    }

    if value, exists := r.Form["bgcolor"]; exists {

        BGColorLoop: 

            for {

                parts := strings.Split(value[0], ",")

                if len(parts) != 3 {

                    break BGColorLoop

                }

                for _, val := range parts {

                    if v, err := strconv.ParseInt(val, 10, 0); err != nil || int(v) > 255 || int(v) < 0 {

                        break BGColorLoop

                    }

                }

                r, _ := strconv.ParseInt(parts[0], 10, 8)

                g, _ := strconv.ParseInt(parts[1], 10, 8)

                b, _ := strconv.ParseInt(parts[2], 10, 8)

                bgcolor = color.RGBA{uint8(r), uint8(g), uint8(b), 255}

                break BGColorLoop

            }

    }

    if value, exists := r.Form["fgcolor"]; exists {

        FGColorLoop: 

            for {

                parts := strings.Split(value[0], ",")

                if len(parts) != 3 {

                    break FGColorLoop

                }

                for _, val := range parts {

                    if v, err := strconv.ParseInt(val, 10, 0); err != nil || int(v) > 255 || int(v) < 0 {

                        break FGColorLoop

                    }

                }

                r, _ := strconv.ParseInt(parts[0], 10, 8)

                g, _ := strconv.ParseInt(parts[1], 10, 8)

                b, _ := strconv.ParseInt(parts[2], 10, 8)

                fgcolor = color.RGBA{uint8(r), uint8(g), uint8(b), 255}

                break FGColorLoop

            }

    }

    lissajous(w, cycles, res, size, frames, delay)

}



func lissajous(out io.Writer, cycles float64, res float64, size int, nframes int, delay int) {

    freq := rand.Float64() * 3.0 

    anim := gif.GIF{LoopCount: nframes}

    phase := 0.0 

    palette := color.Color{bgcolor, fgcolor}

    for i := 0; i<nframes; i++ {

        rect := image.Rect(0, 0, 2*size+1, 2*size+1)

        img := image.NewPaletted(rect, palette)

        for t:=0.0; t<cycles*2*math.Pi; t+=res {

            x := math.Sin(t)

            y := math.Sin(t*freq+phase)

            img.SetColorIndex(size+int(x*float64(size)+0.5), size+int(y*float64(size)+0.5), 1)

        }

        phase += 0.1

        anim.Delay = append(anim.Delay, delay)

        anim.Image = append(anim.Image, img)

    }

    gif.EncodeAll(out, &anim)

}

Code must be correct. To be sure that code is correct, code must be readable.

I took some of your baffling code:

var bgcolor = color.RGBA{0, 0, 0, 255}

var fgcolor = color.RGBA{255, 255, 255, 255}



if value, exists := r.Form["bgcolor"]; exists {

BGColorLoop:

    for {

        parts := strings.Split(value[0], ",")

        if len(parts) != 3 {

            break BGColorLoop

        }

        for _, val := range parts {

            if v, err := strconv.ParseInt(val, 10, 0); err != nil || int(v) > 255 || int(v) < 0 {

                break BGColorLoop

            }

        }

        r, _ := strconv.ParseInt(parts[0], 10, 8)

        g, _ := strconv.ParseInt(parts[1], 10, 8)

        b, _ := strconv.ParseInt(parts[2], 10, 8)

        bgcolor = color.RGBA{uint8(r), uint8(g), uint8(b), 255}

        break BGColorLoop

    }

}

if value, exists := r.Form["fgcolor"]; exists {

FGColorLoop:

    for {

        parts := strings.Split(value[0], ",")

        if len(parts) != 3 {

            break FGColorLoop

        }

        for _, val := range parts {

            if v, err := strconv.ParseInt(val, 10, 0); err != nil || int(v) > 255 || int(v) < 0 {

                break FGColorLoop

            }

        }

        r, _ := strconv.ParseInt(parts[0], 10, 8)

        g, _ := strconv.ParseInt(parts[1], 10, 8)

        b, _ := strconv.ParseInt(parts[2], 10, 8)

        fgcolor = color.RGBA{uint8(r), uint8(g), uint8(b), 255}

        break FGColorLoop

    }

}

I rewrote it (a first draft) in Go:

bgcolor, err := formColor(r, "bgcolor")

if err != nil {

    bgcolor = color.RGBA{0, 0, 0, 255}

}

fgcolor, err := formColor(r, "fgcolor")

if err != nil {

    fgcolor = color.RGBA{255, 255, 255, 255}

}

Where

var errFormColor = errors.New("invalid form color")



func formColor(r *http.Request, colorKey string) (color.RGBA, error) {

    var rgb [3]uint8



    value := r.Form[colorKey]

    if len(value) <= 0 {

        return color.RGBA{}, errFormColor

    }

    parts := strings.SplitN(value[0], ",", len(rgb)+1)

    if len(parts) != len(rgb) {

        return color.RGBA{}, errFormColor

    }

    for i, part := range parts {

        p, err := strconv.ParseUint(part, 10, 8)

        if err != nil {

            return color.RGBA{}, errFormColor

        }

        rgb[i] = uint8(p)

    }



    return color.RGBA{rgb[0], rgb[1], rgb[2], 255}, nil

}