Gwynnie
Gwynnie

Reputation: 504

Go variables being overwritten (bug?)

So bit of a weird one here. My question is, do people get the same results from running my code as I do? And if you do, is it a fault of my code (I'm a python programmer usually), or a bug in golang?

System info: Go version (1.1.2) linux x64 (fedora 19)

Background info on the code: What I'm doing is finding the highest cost route from the top of a triangle to the bottom, this is from project_euler 18 and 67

The bug: I set a variable called pathA, this is an integer list, plus a new int for the new value found from the triangle e.g. 3, 7, 2 append 8 should equal 3, 2, 7, 8 and, it does! ... until I set pathB. pathB gets set correctly however suddenly pathA is the same value as pathB.

tl;dr one variable is being overwritten when I set another

My code is as follows:

package main

import (
    "fmt"
)

func extendPaths(triangle, prePaths [][]int) [][]int {
    nextLine := triangle[len(prePaths)]
    fmt.Println("#####PrePaths: ", prePaths)
    fmt.Println("#####nextLine: ", nextLine)

    postPaths := [][]int{{}}
    for i := 0; i < len(prePaths); i++ {
        route := prePaths[i]
        nextA := nextLine[i]
        nextB := nextLine[i+1]

        fmt.Println("Next A:", nextA, "Next B:", nextB, "\n")
        pathA := append(route, nextA)
        fmt.Println("pathA check#1:", pathA)
        pathB := append(route, nextB)
        fmt.Println("pathA check#2:", pathA, "\n")

        postPaths = append(postPaths, pathA)
        postPaths = append(postPaths, pathB)
    }
    postPaths = postPaths[1:]

    prePaths = [][]int{postPaths[0]}
    for i := 1; i < len(postPaths)-1; i += 2 {
        if getSum(postPaths[i]) > getSum(postPaths[i+1]) {
            prePaths = append(prePaths, postPaths[i])
        } else {
            prePaths = append(prePaths, postPaths[i+1])
        }
    }
    prePaths = append(prePaths, postPaths[len(postPaths)-1])
    return prePaths
}

func getSum(sumList []int) int {
    total := 0
    for i := 0; i < len(sumList); i++ {
        total += sumList[i]
    }
    return total
}

func getPaths(triangle [][]int) {
    prePaths := [][]int{{triangle[0][0]}}
    for i := 0; i < len(triangle)-1; i++ {
        prePaths = extendPaths(triangle, prePaths)
    }
}

func main() {
    triangle := [][]int{{3}, {7, 4}, {2, 4, 6}, {8, 5, 9, 3}}
    getPaths(triangle)
}

This gives the output in my terminal shown below:

#####PrePaths:  [[3]]
#####nextLine:  [7 4]
Next A: 7 Next B: 4

pathA check#1: [3 7]
pathA check#2: [3 7]

#####PrePaths:  [[3 7] [3 4]]
#####nextLine:  [2 4 6]
Next A: 2 Next B: 4

pathA check#1: [3 7 2]
pathA check#2: [3 7 2]

Next A: 4 Next B: 6

pathA check#1: [3 4 4]
pathA check#2: [3 4 4]

#####PrePaths:  [[3 7 2] [3 7 4] [3 4 6]]
#####nextLine:  [8 5 9 3]
Next A: 8 Next B: 5

pathA check#1: [3 7 2 8]
pathA check#2: [3 7 2 5]

Next A: 5 Next B: 9

pathA check#1: [3 7 4 5]
pathA check#2: [3 7 4 9]

Next A: 9 Next B: 3

pathA check#1: [3 4 6 9]
pathA check#2: [3 4 6 3]

Here you can see that for the last 4 times that I set pathA, it is initially set correctly, but then gets overwritten by pathB.

Does anyone have any thoughts on this?

EDIT:

As pointed out by the comments below, what was needed was to make new slices and copy data from the originals. This was done using code from http://blog.golang.org/go-slices-usage-and-internals modified slightly:

func AppendInt(slice []int, data ...int) []int {
    m := len(slice)
    n := m + len(data)
    if n > cap(slice) {
        newSlice := make([]int, (n+1)*2)
        copy(newSlice, slice)
        slice = newSlice
    }
    slice = slice[0:n]
    copy(slice[m:n], data)
    return slice
}

I also changed the code on the other side, where I created the slices pathA and pathB. This changed to:

for i := 0; i < len(prePaths); i++ {

    nextA := nextLine[i]
    nextB := nextLine[i+1]

    pathA := AppendInt(prePaths[i], nextA)
    pathB := AppendInt(prePaths[i], nextB)

    postPaths = append(postPaths, pathA)
    postPaths = append(postPaths, pathB)
}

EDIT2:

It's quite early in the morning here, and I flat out made a mistake in my first edit, I did not fully understand your solution, after a bit of hacking I got there in the end:

This code does not work (pathA gets overwritten):

for i := 0; i < len(prePaths); i++ {

    nextA := nextLine[i]
    nextB := nextLine[i+1]

    pathA := append(prePaths[i], nextA)
    pathB := append(prePaths[i], nextB)

    postPaths = append(postPaths, pathA)
    postPaths = append(postPaths, pathB)
}

This code also does not work (pathA gets overwritten):

for i := 0; i < len(prePaths); i++ {

    newRoute := make([]int, len(prePaths[i]), (cap(prePaths[i])+1)*2)
    copy(newRoute, prePaths[i])

    nextA := nextLine[i]
    nextB := nextLine[i+1]

    pathA := append(newRoute, nextA)
    pathB := append(newRoute, nextB)

    postPaths = append(postPaths, pathA)
    postPaths = append(postPaths, pathB)
}

However, if I mix the 2 scenarios above into the code below, it works fine (pathA does not get overwritten):

for i := 0; i < len(prePaths); i++ {

    newRoute := make([]int, len(prePaths[i]), (cap(prePaths[i])+1)*2)
    copy(newRoute, prePaths[i])

    nextA := nextLine[i]
    nextB := nextLine[i+1]

    pathA := append(newRoute, nextA)
    pathB := append(prePaths[i], nextB)

    postPaths = append(postPaths, pathA)
    postPaths = append(postPaths, pathB)
}

So, my solution was to make a copy of the array, and have them both use different ones.

Upvotes: 4

Views: 4239

Answers (1)

James Henstridge
James Henstridge

Reputation: 43949

A slice is basically a structure consisting of 3 things:

  1. A pointer to an array of the elements in the slice
  2. The length of that array (the "capacity")
  3. The number of elements actually stored in the array (the "length")

When you run the following code:

append(x, element)

It does the following:

  1. Check if extending the slice will exceed the capacity of the underlying array. If so, allocate a larger one and copy the existing elements to the new array, and update the capacity.
  2. Write the new element (or elements) to the end of the array and update the length.
  3. Return the new slice.

In your code, you have the following:

pathA := append(route, nextA)
pathB := append(route, nextB)

Now there are two possibilities here:

  1. len(route) == cap(route), and a new backing array will be allocated, with pathA and pathB having independent values.
  2. len(route) < cap(route), so pathA and pathB end up sharing the same backing array. The last element in the array will be nextB, since that operation was run second.

It seems that the first case is true for the first few iterations of your loop, after which you hit the second case. You could avoid this by manually making a copy for one of your paths (allocate a slice with make(), and then use copy() to copy the old data).

Upvotes: 5

Related Questions