3
\$\begingroup\$

I wrote a little wrapper for Google Static Maps API so that I do not pull down from Google Maps if I have already retrieved the address. The below code works, but I wrote it in a quick fashion - not really paying attention to the best practices of golang.

Several questions I have about this code:
- How can I improve the v := url.Values{} section?
- Is there a better method to the for i := 0; i < 30; i++ loop?

package main

import (
    "crypto/md5"
    "encoding/hex"
    "errors"
    "fmt"
    "io/ioutil"
    "net/http"
    "net/url"
    "os"
)

var apiKey = os.Getenv("API_KEY")

// loadMap generates a new map or returns a cached one
func loadMap(address string, title string) ([]byte, error) {
    mapHost := "http://maps.googleapis.com/maps/api/staticmap"

    v := url.Values{}
    v.Add("center", address)
    v.Add("zoom", "15")
    v.Add("scale", "2")
    v.Add("size", "400x350")
    v.Add("markers", address)
    v.Add("sensor", "false")
    v.Add("key", apiKey)
    mapURL := mapHost + "?" + v.Encode()

    filename := title + ".png"
    for i := 0; i < 30; i++ {
        body, err := ioutil.ReadFile(filename)
        if err == nil {
            return body, nil
        }

        if i == 0 {
            fmt.Println("Getting map for " + address + " and saving it to " + filename)
            fmt.Println("Map URL: " + mapURL)
            httpClientGetMap, _ := http.Get(mapURL)
            body, _ := ioutil.ReadAll(httpClientGetMap.Body)
            ioutil.WriteFile(filename, []byte(body), 0600)
        }

        i++
    }

    return nil, errors.New("Failed to get an image")
}

// viewHandler returns the map image
func viewHandler(w http.ResponseWriter, r *http.Request) {

    address := r.URL.Query().Get("address")

    md5 := md5.Sum([]byte(address))
    title := hex.EncodeToString(md5[:])
    p, err := loadMap(address, title)

    if err != nil {
        fmt.Println(err)
        w.WriteHeader(http.StatusGatewayTimeout)
        return
    }

    w.Header().Set("content-type", "image/png")
    w.Write(p)
}

func main() {
    http.HandleFunc("/getmap", viewHandler)

    http.ListenAndServe(":8080", nil)
}
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Improve the for loop

The loadMap method is really hard to read, and is not verry efficient:

Currently, if a file doesn't exists, here are the steps followed by the method:

  • try to read the content of a file to see if it exists
  • send a request to google map, and write the response to a file
  • read the content of the new file

but we could merge just return the response instead of reading the file twice ! Also, it would be better to have a map for keeping records of saved map. To do this, we create a server that implement the ServerHTTP method:

  type server struct {
    mux        *http.ServeMux
    fetchedMap map[string]string
  }

  func (s *server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
      s.mux.ServeHTTP(w, r)
  }

Now, let's rewritte the loadMap method:

  func (s *server) loadMap(address string, title string) ([]byte, error) {

    if filename, ok := s.fetchedMap[title]; ok {
        return ioutil.ReadFile(filename)
    }
    mapHost := "http://maps.googleapis.com/maps/api/staticmap"

    v := url.Values{}
    v.Add("center", address)
    v.Add("zoom", "15")
    v.Add("scale", "2")
    v.Add("size", "400x350")
    v.Add("markers", address)
    v.Add("sensor", "false")
    v.Add("key", apiKey)
    mapURL := mapHost + "?" + v.Encode()

    fmt.Println("Getting map for " + address + " and saving it to " + filename)
    fmt.Println("Map URL: " + mapURL)
    httpClientGetMap, _ := http.Get(mapURL)
    body, _ := ioutil.ReadAll(httpClientGetMap.Body)
    ioutil.WriteFile(filename, []byte(body), 0600)
    return body, nil
  }

The steps are now:

  • check if requested address has already been fetched, and if yes, return file content
  • if no fetch map from google map and write it to a file
  • return the map

General advices

  • use constant for mapHost and other constant parameters
  • use fmt.Printf("%v", str) instead of fmt.Println(" " + str) because it's easier to read
  • Always check the error ! Silently discarding errors is really dangerous
  • avoid useless conversion: body is already a slice of byte

Here is the new version of the code:

package main

import (
    "crypto/md5"
    "encoding/hex"
    "fmt"
    "io/ioutil"
    "net/http"
    "net/url"
    "os"
)

var apiKey = os.Getenv("API_KEY")

const (
    baseURL = "http://maps.googleapis.com/maps/api/staticmap"
    zoom    = "15"
    scale   = "2"
    size    = "400x350"
)

func (s *server) loadMap(address string, title string) ([]byte, error) {

    if filename, ok := s.fetchedMap[title]; ok {
        return ioutil.ReadFile(filename)
    }
    v := url.Values{
        "center":  {address},
        "zoom":    {zoom},
        "scale":   {scale},
        "size":    {size},
        "markers": {address},
        "sensor":  {"false"},
        "key":     {apiKey},
    }
    mapURL := baseURL + "?" + v.Encode()
    filename := title + ".png"

    fmt.Printf("Getting map for adress %s and saving it to %s\nMap URL: %s\n", address, filename, mapURL)

    httpClientGetMap, err := http.Get(mapURL)
    if err != nil {
        return nil, err
    }
    body, err := ioutil.ReadAll(httpClientGetMap.Body)
    if err != nil {
        return nil, err
    }
    err = ioutil.WriteFile(filename, body, 0600)
    if err != nil {
        return nil, err
    }
    s.fetchedMap[title] = filename
    return body, nil
}

func (s *server) viewHandler(w http.ResponseWriter, r *http.Request) {

    address := r.URL.Query().Get("address")

    checksum := md5.Sum([]byte(address))
    title := hex.EncodeToString(checksum[:])
    mapByte, err := s.loadMap(address, title)
    if err != nil {
        fmt.Println(err)
        w.WriteHeader(http.StatusGatewayTimeout)
        return
    }
    w.Header().Set("content-type", "image/png")
    w.Write(mapByte)
}

type server struct {
    mux        *http.ServeMux
    fetchedMap map[string]string
}

func (s *server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
    s.mux.ServeHTTP(w, r)
}

func main() {

    s := &server{
        mux:        http.DefaultServeMux,
        fetchedMap: map[string]string{},
    }
    s.mux.HandleFunc("/getmap", s.viewHandler)
    http.ListenAndServe(":8080", s)
}
\$\endgroup\$

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.