Making two similar functions generic

Question

I have two functions in Go that do almost the same. They take a slice of structs which has an 'ID' field, and re-sort it into a map indexed by that field. They then append it to a field of another struct, which is also identified by the ID.

The two functions do the same, but append to two different fields in the struct. I want to make the methods generic, but I am not sure how to do it. I would expect it could be done using a pointer, but I'm not sure how.

Function 1:

func addPremiereDatesToMovies(m []Movie, pd []PremiereDate) ([]Movie, error) {
    pds := make(map[int64][]PremiereDate)

    // Key-index the premiere-date array for easier lookup
    for _, value := range pd {
        pds[value.Id] = append(pds[value.Id], value)
    }

    // Append premiere dates to movies where such exist
    for key, value := range m {
        if _, ok := pds[value.Id]; !ok {  // <-- How to make this generic?
            m[key].PremiereDates = []PremiereDate{}
            continue
        }

        m[key].PremiereDates = pds[value.Id]
    }

    return m, nil
}

Function 2:

func addGenresToMovies(m []Movie, g []Genre) ([]Movie, error) {
    gs := make(map[int64][]Genre)

    // Key-index the genre array for easier lookup
    for _, value := range g {
        gs[value.Id] = append(gs[value.Id], value)
    }

    // Append genres to movies where such exist
    for key, value := range m {
        if _, ok := gs[value.Id]; !ok { // <-- How to make this generic?
            m[key].Genres = []Genre{}
            continue
        }

        m[key].Genres = gs[value.Id]
    }

    return m, nil
}

As it seems, they look quite similar. I am able to sort of do it, except I cannot figure out how to generically describe the "value.Id" field on line 11.

Thank you.

i think for the code you presented in your question there is little benefit of having a generic function. two separate functions are perfectly fine, readable and type safe. you can save a few lines of code by not creating empty slice for missing ids (just use nil instead). — kostya, Jul 08 '16 at 12:48
I've tried to compose a solution but it seems your code is flowed. You are building `pds` & `gds` maps that are keyed by PremierDate and Genre `Id` but then you are looking up by movie ID: `gs[value.Id]`. Does not make any sense to me. Also note instead of `m[key].Genres = ` you can do `value.Genres = ` — Alexander Trakhimenok, Jul 11 '16 at 15:12

score 3 · Answer 1 · edited May 23 '17 at 12:22

While I agree with kostya that having it as separate functions is perfectly fine, it might become less maintainable if you want to add 20+ items to a movie - like cast ,revenues_per_country, etc. You'd be left with 20+ functions.

The following function would compress it into one function, but you'll need to add different case sections for each field you want to add.

To use it, simply drop it in place of your current calls:

Usage: movies = addStuffToMovies(movies, genres)

// addStuffToMovies adds fields to a movie where the ID matches
func addStuffToMovies(movies []Movie, stuff interface{}) []Movie {

    // Go through the movies list
    for current, movie := range movies {

        // check the type and append based on that
        switch v := stuff.(type) {

        // This is the section you'll need to duplicate for each type of field
        case []Genre:
            for _, item := range v {
                // if it matches, append it to Genres
                if item.Id == movie.Id {
                    movies[current].Genres = append(movies[current].Genres, item)
                }
            }

        // This is the section you'll need to duplicate for each type of field
        case []PremiereDate:
            for _, item := range v {
                // if it matches, append it to PremiereDates
                if item.Id == movie.Id {
                    movies[current].PremiereDates = append(movies[current].PremiereDates, item)
                }
            }
        }
    }
    return movies
}

You could probably take it a step further by adding an interface, then instead of stuff interface{} you'll have something like []FieldInterface with calls like GetID(), GetType() instead of the different case sections. I think you'll still need to do type checking somewhere to actually assign it to the movie. This will probably be way to much effort when a switch like above will work fine.

Making two similar functions generic

1 Answers1