1
\$\begingroup\$

This is my first Go project. Coming from a Java developer background, this code just smells very bad. But, cannot figure out better way to do this.

It's a small app to find suggested flight destinations based on one or more origins (by their airport-codes, called codesbelow). Basically finding flights that have the same destination and about the same arrival times.

// SuggestDestinations takes a list of origin airport codes, an earliest arrival time and a latest arrival time and
// returns a map, where the destination is the key, and the suggested flights.
func (service *FlightService) SuggestDestinations(codes []string, earliestArrival, latestArrival time.Time) (destinationMap map[string][]internal.Flight, err error) {
    rows, err := service.db.Query(selectFlightsSQL, pq.Array(codes), earliestArrival, latestArrival)
    if err != nil {
        return nil, err
    }
    defer rows.Close()
    var m map[string][]internal.Flight
    m = make(map[string][]internal.Flight)
    for rows.Next() {
        var a internal.Flight
        var origin, destination, airline string
        if e := rows.Scan(&origin, &destination, &airline, &a.Departure, &a.Arrival); err != nil {
            return nil, e
        }
        a.Origin, err = service.GetAirport(origin)
        a.Destination, err = service.GetAirport(destination)
        a.Airline, err = service.GetAirline(airline)
        // Filter out destinations which is one of the origins
        if !contains(codes, a.Destination.Code) {
            m[a.Destination.Code] = append(m[a.Destination.Code], a)
        }
    }

    // Delete destinations for which there are not flights from every given origin
    for k := range m {
        if !containsAllCodes(m[k], codes) {
            delete(m, k)
        }
    }

    return m, nil
}

Where containsAllCodes looks like this:

func containsAllCodes(flights []internal.Flight, codes []string) bool {
containsAllOrigin := true
for _, c := range codes {
    containsOrigin := false
    for _, f := range flights {
        containsOrigin = containsOrigin || f.Origin.Code == c
    }
    containsAllOrigin = containsAllOrigin && containsOrigin
    }
    return containsAllOrigin
}
\$\endgroup\$
0
\$\begingroup\$

... this code just smells very bad. But, cannot figure out better way to do this.

Let's look at a more complete, more fragrant implementation.


For data, you are using PostgreSQL: db.Query(selectFlightsSQL, pq.Array(codes), ...). Therefore, define data using the SQL Data Definition Language (DDL). For example,

CREATE TABLE airlines (
    airlinecode char(3) NOT NULL,
    airlinename varchar(64) NOT NULL,
    PRIMARY KEY (airlinecode),
    UNIQUE (airlinename)
);

CREATE TABLE airports (
    airportcode char(3) NOT NULL,
    airportname varchar(64) NOT NULL,
    timezonename varchar(64) NOT NULL,
    PRIMARY KEY (airportcode),
    UNIQUE (airportname)
);

CREATE TABLE flights (
    airlinecode char(3) NOT NULL,
    flightno integer NOT NULL,
    departtime timestamptz NOT NULL, -- local time
    origincode char(3) NOT NULL,
    arrivetime timestamptz NOT NULL, -- local time
    destinationcode char(3) NOT NULL,
    PRIMARY KEY (airlinecode, flightno, departtime),
    FOREIGN KEY (airlinecode) REFERENCES airlines,
    FOREIGN KEY (origincode) REFERENCES airports,
    FOREIGN KEY (destinationcode) REFERENCES airports
);

Access the data using the SQL Data Manipulation Language (DML). For example,

    -- $1  origincode list
    -- $2  origincode list length
    -- $3  earliest arrivaltime
    -- $4  latest arrivaltime
    SELECT destinationcode, airportd.airportname AS destinationname, arrivetime, airportd.timezonename AS arrivezone,
        origincode, airporto.airportname AS originname, departtime, airporto.timezonename AS departzone,
        destinations.airlinecode, airlines.airlinename, flightno

    FROM (

        SELECT destinationcode, origincode, airlinecode, flightno, departtime, arrivetime,
            count(origincode) OVER (PARTITION BY destinationcode) AS origins
        FROM flights
        WHERE arrivetime >= $3 AND arrivetime <= $4
            AND origincode = ANY($1)
            AND destinationcode != ANY($1)

    ) AS destinations 
    LEFT JOIN airlines ON airlines.airlinecode = destinations.airlinecode
    LEFT JOIN airports AS airportd ON airportd.airportcode = destinationcode
    LEFT JOIN airports AS airporto ON airporto.airportcode = origincode

    WHERE origins = $2 

    ORDER BY destinationcode, arrivetime, airlinecode, flightno, departtime
    ;

Consider optimizations such as indices.

Data access is implemented in SQL for any programming language that implements an interface to PostgreSQL (and other relational DBMS`s), not just Go.


In Go, the remaining algorithmic, procedural task is simple: convert the SQL rows to a Go hash map indexed by destination code. For example,

package main

import (
    "database/sql"
    "time"

    "github.com/lib/pq"
    _ "github.com/lib/pq"
)

type Flight struct {
    DestinationCode string
    DestinationName string
    ArriveTime      time.Time
    ArriveZone      string
    OriginCode      string
    OriginName      string
    DepartTime      time.Time
    DepartZone      string
    AirlineCode     string
    AirlineName     string
    FlightNo        int
}

func destinations(db *sql.DB, origins []string, earliest, latest time.Time) (map[string][]Flight, error) {
    query := `
        -- $1  origincode list
        -- $2  origincode list length
        -- $3  earliest arrivaltime
        -- $4  latest arrivaltime
       SELECT destinationcode, airportd.airportname AS destinationname, arrivetime, airportd.timezonename AS arrivezone,
            origincode, airporto.airportname AS originname, departtime, airporto.timezonename AS departzone,
            destinations.airlinecode, airlines.airlinename, flightno

        FROM (

            SELECT destinationcode, origincode, airlinecode, flightno, departtime, arrivetime,
                count(origincode) OVER (PARTITION BY destinationcode) AS origins
            FROM flights
            WHERE arrivetime >= $3 AND arrivetime <= $4
                AND origincode = ANY($1)
                AND destinationcode != ANY($1)

        ) AS destinations 
        LEFT JOIN airlines ON airlines.airlinecode = destinations.airlinecode
        LEFT JOIN airports AS airportd ON airportd.airportcode = destinationcode
        LEFT JOIN airports AS airporto ON airporto.airportcode = origincode

        WHERE origins = $2 

        ORDER BY destinationcode, arrivetime, airlinecode, flightno, departtime
        ;
        `

    dst := make(map[string][]Flight)
    rows, err := db.Query(query, pq.Array(origins), len(origins), earliest, latest)
    if err != nil {
        return nil, err
    }
    defer rows.Close()
    for rows.Next() {
        var flt Flight
        err := rows.Scan(
            &flt.DestinationCode,
            &flt.DestinationName,
            &flt.ArriveTime,
            &flt.ArriveZone,
            &flt.OriginCode,
            &flt.OriginName,
            &flt.DepartTime,
            &flt.DepartZone,
            &flt.AirlineCode,
            &flt.AirlineName,
            &flt.FlightNo,
        )
        if err != nil {
            return nil, err
        }
        dst[flt.DestinationCode] = append(dst[flt.DestinationCode], flt)
    }
    err = rows.Err()
    if err != nil {
        return nil, err
    }
    return dst, nil
}

func main() {}

The XY problem is asking about your attempted solution rather than your actual problem: The XY Problem. Therefore, without a problem, we have no way of knowing whether a solution is correct. There are no use cases or examples.


TODO: Domain knowledge is important. Airline arrival and departure times are local times. Any solution needs to be verified and tested across time zones and for standard and daylight saving times.

\$\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.