CODEWITHSUNDEEP
goslice
Andrey Larin
Andrey Larin

Reputation: 94

Slice merge in golang recommendation

Is there a way to make this golang code shorter?

func MergeSlices(s1 []float32, s2 []int32) []int {
    var slice []int
    for i := range s1 {
        slice = append(slice, int(s1[i]))
    }
    for i := range s2 {
        slice = append(slice, int(s2[i]))
    }
    return slice
}

Upvotes: 3

Views: 5171

Answers (4)

Vince
Vince

Reputation: 31

var s1 []int
var s2 []int

newSlice = append(s1, s2...)

Upvotes: 3

Adrian
Adrian

Reputation: 46442

The code can't get any shorter, but that's a goal of dubious value to begin with; it's not overly verbose as-is. You can, however, likely improve performance by eliminating the intermediate allocations. Every time you call append, if the target slice doesn't have enough space, it expands it, guessing at the necessary size since you haven't told it how much space it will need.

The simplest would just be to presize your target slice (replace var slice []int with slice := make([]int, 0, len(s1) + len(s2)); that way the appends never have to expand it. Setting the second parameter to 0 is important, that sets the length to zero, and the capacity to the total size needed, so that your appends will work as expected.

Once you've presized it though, you can get rid of the appends entirely, and directly set each index:

func MergeSlices(s1 []float32, s2 []int32) []int {
    slice := make([]int, len(s1) + len(s2))
    for i,v := range s1 {
        slice[i] = int(v)
    }
    for i,v := range s2 {
        slice[i+len(s1)] = int(v)
    }
    return slice
}

Playground link

Upvotes: 0

icza
icza

Reputation: 417662

You can't eliminate the loops to convert each element to int individually, because you can't convert whole slices of different element types. For explanation, see this question: Type converting slices of interfaces in go

The most you can do is use named result type, and a for range with 2 iteration values, where you can omit the first (the index) by assigning it to the blank identifier, and the 2nd will be the value:

func MergeSlices(s1 []float32, s2 []int32) (s []int) {
    for _, v := range s1 {
        s = append(s, int(v))
    }
    for _, v := range s2 {
        s = append(s, int(v))
    }
    return
}

But know that your code is fine as-is. My code is not something to always follow, it was to answer your question: how to make your code shorter. If you want to improve your code, you could start by looking at its performance, or even refactoring your code to not end up needing to merge slices of different types.

Upvotes: 5

peterSO
peterSO

Reputation: 166598

Your code should be correct, maintainable, readable, and reasonably efficient. Note that shortness of code is not one of the important goals. For good reason, Stack Exchange has another site for Code Golf questions: Programming Puzzles & Code Golf.

Your code could be improved; it's inefficient. For example, merging two len(256) slices,

BenchmarkMergeSlices      200000      8350 ns/op    8184 B/op     10 allocs/op

Here's a more efficient (and longer) version:

BenchmarkMergeSlices      300000      4420 ns/op    4096 B/op      1 allocs/op

.

func MergeSlices(s1 []float32, s2 []int32) []int {
    slice := make([]int, 0, len(s1)+len(s2))
    for i := range s1 {
        slice = append(slice, int(s1[i]))
    }
    for i := range s2 {
        slice = append(slice, int(s2[i]))
    }
    return slice
}

Use the Go Code Review Comments for Named Result Parameters. For example: "Don't name result parameters just to avoid declaring a var inside the function; that trades off a minor implementation brevity at the cost of unnecessary API verbosity. Clarity of docs is always more important than saving a line or two in your function."

Upvotes: 3

Related Questions