-1

If I have some code, like the example below, that fetches an image from a link and then saves it to disk, what is the best way to pass around the image data?

I thought about using ioutil.ReadAll(res.Body) to convert it to []byte but passing that around seems expensive, although I can't tell from the documentation whether or not it returns a slice or an array. I also tried returning a pointer to res.Body, a *io.ReadCloser type, but I couldn't figure out how to properly call the .Close() method on the pointed to interface.

I understand that moving the save code into the FetchImage function would probably be the easiest way to solve this but I would like to have these pieces separate if possible.

type ImageData struct {
    Data      io.ReadCloser
    Name      string
}

func FetchImage(url string) (io.ReadCloser, error) {
    res, err := http.Get(url)
    if err != nil {
        return nil, err
    }
    return res.Body, nil
}

func Save(data *ImageData) error {
    defer data.Data.Close()
    file, err := os.Create(data.Name)
    defer file.Close()
    if err != nil {
        return err
    }
    _, err = io.Copy(file, data.Data)
    if err != nil {
        return err
    }
    return nil
}

func main() {
    body, err := fetcher.FetchImage("https://imgur.com/asdf.jpg")
    if err != nil {
        panic(err)
    }
    imageData := ImageData{body, "asdf.jpg"}
    saver := Saver{config.BaseDir, 1}
    err = saver.Save(&imageData)
    if err != nil {
        panic(err)
    }
}

Additionally, I am very new to Go so if there's anything in this code that looks bad please let me know.

Wilfred
  • 799
  • 2
  • 16
  • 26

2 Answers2

2

Use ioutil.ReadAll. The function returns a slice of bytes.

Slices are efficient to pass around. A slice is a pointer to the backing array, a length and a capacity.

type ImageData struct {
    Data      []byte
    Name      string
}

func FetchImage(url string) ([]byte, error) {
    res, err := http.Get(url)
    if err != nil {
        return nil, err
    }
    defer resp.Body.Close()
    if res.StatusCode != 200 {
        return nil, fmt.Errorf("%s: %d", url, res.StatusCode)
    }
    return ioutil.ReadAll(resp.Body)
}

func Save(data *ImageData) error {
    file, err := os.Create(data.Name)
    if err != nil {
        return err
    }
    defer file.Close()
    _, err := file.Write(data.Data)
    return err
}

You can also pass around the response body, but use caution. The response body must be closed to release the underlying connection. The code in the question does close the response body, but it's difficult to see because the response body is passed down the function where it's closed.

Charlie Tumahai
  • 113,709
  • 12
  • 249
  • 242
  • Thanks for the response. Isn't it a little redundant though because then you would need to make a new Reader to pass to io.Copy when res.Body already satisfies that interface? – Wilfred Mar 19 '16 at 23:36
  • Call Write directly instead of using io.Copy: `file.Write(data)` – Charlie Tumahai Mar 19 '16 at 23:38
1

In Go all arrays have a size specifier, and will look like [8]byte. Slices will not have this and look like []byte. Slices internally just store a pointer to the data, the length of the slice, and the capacity of the slice. This is just 24 bytes on a 64 bit system, so you don't have to worry about passing it around.

However, I would recommend to return a *ImageData from FetchImage as it already has metadata like the name of the image.

As to why you cant take a pointer to a interface, there is a post here on SO that explains why.

Also, in Save you defer file.Close() before checking for a error. You should swap this because file will be nil if there is a error, and will probably segfault.

Community
  • 1
  • 1
Abex
  • 300
  • 5
  • 11