9
\$\begingroup\$

I am learning Go and wrote a small application that queries the Skyscanner API as my first attempt at learning. I was hoping someone more expert than me in the language could look over it at broad strokes and let me know if I'm on the right track. I really appreciate any feedback I can get!

Also, I know this is a fair amount to read over. Feedback on even one or two of the files is appreciated.

GitHub repository

main.go

package main

import (
    "fmt"
    "log"
    "os"
    "text/tabwriter"

    "github.com/sdstolworthy/go-fly/skyscanner"
)

func main() {
    fmt.Println()
    params := skyscanner.Parameters{
        Adults:           1,
        Country:          "US",
        Currency:         "USD",
        Locale:           "en-US",
        OriginPlace:      "SLC-sky",
        DestinationPlace: "BNA-sky",
        OutbandDate:      "anytime",
        InboundDate:      "anytime",
    }

    for _, v := range DestinationAirports {
        params.DestinationPlace = v
        fmt.Println(v)
        SkyscannerQuotes := skyscanner.BrowseQuotes(params)

        w := tabwriter.NewWriter(os.Stdout, 0, 0, 3, ' ', 0)
        quote, err := SkyscannerQuotes.LowestPrice()
        if err != nil {
            log.Printf("%v\n\n", err)
            continue
        }
        fmt.Fprintf(w, "Price:\t$%v\nDeparture:\t%v\nReturn:\t%v\t\n\n", quote.Price, quote.DepartureDate, quote.InboundDate)
    }
}

destinations.go

package main

// DestinationAirports is an array of all airports that should be queried
var DestinationAirports = [...]string{
    "CDG-sky",
    "BNA-sky",
    "LHR-sky",
    "ZAG-sky",
    "SAN-sky",
    "MEX-sky",
}

skyscanner/config.go

package skyscanner

import (
    "io/ioutil"
    "log"

    "gopkg.in/yaml.v2"
)

// Config contains application secrets
type Config struct {
    MashapeKey  string `yaml:"mashape_key"`
    MashapeHost string `yaml:"mashape_host"`
    BaseURL     string `yaml:"base_url"`
}

func (c *Config) getConfig() *Config {
    yamlFile, err := ioutil.ReadFile("conf.yaml")
    if err != nil {
        log.Printf("yamlFile.Get err   #%v ", err)
    }
    err = yaml.Unmarshal(yamlFile, c)
    if err != nil {
        log.Fatalf("Unmarshal: %v", err)
    }
    return c
}

skyscanner/network.go

package skyscanner

import (
    "encoding/json"
    "fmt"
    "io/ioutil"
    "log"
    "net/http"
    "time"
)

func prettyPrint(apiResponse Response) {
    fmt.Printf("%+v\n", apiResponse.Quotes)
}

func parseResponse(response *http.Response) Response {
    body, readErr := ioutil.ReadAll(response.Body)
    if readErr != nil {
        log.Fatal(readErr)
    }

    apiResponse := Response{}
    jsonErr := json.Unmarshal(body, &apiResponse)
    if jsonErr != nil {
        log.Fatal(jsonErr)
    }
    return apiResponse
}

func getRequest(url string) *http.Request {
    req, err := http.NewRequest(http.MethodGet, url, nil)
    var config Config
    config.getConfig()
    req.Header.Set("X-Mashape-Key", config.MashapeKey)
    req.Header.Set("X-Mashape-Host", config.MashapeHost)

    if err != nil {
        log.Fatal(err)
    }
    return req
}

func getClient() *http.Client {
    return &http.Client{
        Timeout: time.Second * 2,
    }
}

func formatURL(path string) string {
    var c Config
    c.getConfig()
    baseURL := c.getConfig().BaseURL

    return fmt.Sprintf("%v%v", baseURL, path)
}

func get(url string) *http.Response {
    res, getErr := getClient().Do(getRequest(url))

    if getErr != nil {
        log.Fatal(getErr)
    }

    return res

}

/*
BrowseQuotes stub
*/
func BrowseQuotes(parameters Parameters) Response {
    browseQuotes := formatURL(fmt.Sprintf("browsequotes/v1.0/%v/%v/%v/%v/%v/%v/%v",
        parameters.Country,
        parameters.Currency,
        parameters.Locale,
        parameters.OriginPlace,
        parameters.DestinationPlace,
        parameters.OutbandDate,
        parameters.InboundDate,
    ))
    res := get(browseQuotes)
    return parseResponse(res)
}

skyscanner/parameters.go

package skyscanner

// Parameters generic request type for skyscanner api
type Parameters struct {
    Country          string `json:"country"`
    Currency         string `json:"currency"`
    Locale           string `json:"locale"`
    OriginPlace      string `json:"originPlace"`
    DestinationPlace string `json:"destinationPlace"`
    OutbandDate      string `json:"outboundDate"`
    Adults           int    `json:"adults"`
    InboundDate      string `json:"inboundDate"`
}

skyscanner/response.go

package skyscanner

import "errors"

// Response generic response from skyscanner api
type Response struct {
    Quotes []struct {
        QuoteID     int     `json:"QuoteId"`
        MinPrice    float64 `json:"MinPrice"`
        Direct      bool    `json:"Direct"`
        OutboundLeg struct {
            CarrierIds    []int  `json:"CarrierIds"`
            OriginID      int    `json:"OriginId"`
            DestinationID int    `json:"DestinationId"`
            DepartureDate string `json:"DepartureDate"`
        } `json:"OutboundLeg"`
        InboundLeg struct {
            CarrierIds    []int  `json:"CarrierIds"`
            OriginID      int    `json:"OriginId"`
            DestinationID int    `json:"DestinationId"`
            DepartureDate string `json:"DepartureDate"`
        } `json:"InboundLeg"`
        QuoteDateTime string `json:"QuoteDateTime"`
    } `json:"Quotes"`
    Places []struct {
        PlaceID        int    `json:"PlaceId"`
        IataCode       string `json:"IataCode"`
        Name           string `json:"Name"`
        Type           string `json:"Type"`
        SkyscannerCode string `json:"SkyscannerCode"`
        CityName       string `json:"CityName"`
        CityID         string `json:"CityId"`
        CountryName    string `json:"CountryName"`
    } `json:"Places"`
    Carriers []struct {
        CarrierID int    `json:"CarrierId"`
        Name      string `json:"Name"`
    } `json:"Carriers"`
    Currencies []struct {
        Code                        string `json:"Code"`
        Symbol                      string `json:"Symbol"`
        ThousandsSeparator          string `json:"ThousandsSeparator"`
        DecimalSeparator            string `json:"DecimalSeparator"`
        SymbolOnLeft                bool   `json:"SymbolOnLeft"`
        SpaceBetweenAmountAndSymbol bool   `json:"SpaceBetweenAmountAndSymbol"`
        RoundingCoefficient         int    `json:"RoundingCoefficient"`
        DecimalDigits               int    `json:"DecimalDigits"`
    } `json:"Currencies"`
}

// Prices returns an array of the lowest prices for a route and date
func (r *Response) Prices() []float64 {
    priceList := make([]float64, len(r.Quotes))
    for i, v := range r.Quotes {
        priceList[i] = v.MinPrice
    }
    return priceList
}

// A QuoteSummary is a summary of a outbound trip with its price and date
type QuoteSummary struct {
    Price         float64
    DepartureDate string
    InboundDate   string
}

// LowestPrice prints a list of the lowest prices and their accompanying dates
func (r *Response) LowestPrice() (*QuoteSummary, error) {
    quote := QuoteSummary{
        99999999,
        "",
        "",
    }
    for _, v := range r.Quotes {
        if v.MinPrice < quote.Price {
            quote.Price = v.MinPrice
            quote.DepartureDate = v.OutboundLeg.DepartureDate
            quote.InboundDate = v.InboundLeg.DepartureDate
        }
    }
    if quote.Price == 99999999 {
        return nil, errors.New("No quote found")
    }
    return &quote, nil

}
\$\endgroup\$
2
\$\begingroup\$

(I only looked at the GitHub repository ...)

Ah you've got go vet already on, that's why it's not finding anything, great!

Dockerfile looks good too. You might want to specify an exact Go version just in case.

main.go looks fine to me. Only the unused return value from router.Run() would pop up depending on your linter settings, which you could explicitly ignore with _ = router.Run(), but I see it's being logged in case of errors already.

airports.go has a few obvious comments that therefore don't add much information. Also f should be closed. Not sure what happens if you can't open the file? Also the constructed path there might not always work, I'd argue that a command-line flag specifying the file directly, or perhaps just a relative path if it's always called from the main directory might be a bit safer. Finally each struct is saved individually - I'm guessing this is not a problem so far, but you might want some batching and transaction safety at some point.

db.go imports SQLite and PostgreSQL, but the connection opening is using "postgres" only, consider moving that into the configuration object too, or otherwise get rid of SQLite?

The fmt.Println should probably be a "proper" logger before you've to convert more of those calls later on.

airports.go has all methods that can return an error, but don't have a single one that actually does in any situation. I'd get rid of the error return value if it's not being used. Instead of upper(...) LIKE upper(...) consider using ILIKE. Now that I think about it, also consider adding some default LIMIT's to limit the potential for mistakes/misuse.

airportsController.go etc. use a global variable environment - that's usually not a great idea since it makes it a bit harder to work with, plus, nothing prevents you from putting that into the controller structure and making e.g. seachAirports a method instead.

I'm not familiar with gin, so context.JSON etc. look a bit weird to me, but nothing too bad. Consider though logging the database error for debug purposes! It might be that the connection got dropped, or any other of the big pool of possible failures and just returning "No results found" will not help you figuring that one out after the fact. N.b. it looks like if the caller specifies both iataCode and cityName that the HTTP response will be attempted to be written twice, that looks wrong to me, even if the framework catches that for you.

destinations.go, I had to look up the syntax again, the [...] doesn't really buy you anything, does it.

quoteController.go is a bit more interesting again, saveQuotes should set up a transaction I think?


Alright, so the skyscanner package again uses a global configuration and again I'd advise against that.

analytics.go looks okay as long as your floating point values don't get too big I think. Otherwise you'd have to search for the proper algorithms and/or use a library.

http.go, hmm, looks a bit more odd, the logging isn't consistent (more fmt.Print...), also printing of values without indication where it's coming from; I'd also expect that you'd get rate-limited at some point. Maybe doing only a few requests at the same time plus handling errors more gracefully (that is, retrying automatically) would improve this. Also prettyPrint is unused.

response.go looks fine, except in LowestPrice I rally dislike the placeholder value 99999999 - of course it's somewhat common practice, but simply looking at len(r.Quotes) would immediately tell you whether there were any quotes at all!


Overall looks good to me, you could then also add some tests maybe.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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