Decoding empty list/map in stream into a nil list/map destination should update destination to empty list/map

[zond]

The following code:

package main

import (
    "fmt"
    "github.com/zond/go/codec"
)

func main() {
    bincHandle := new(codec.BincHandle)
    var b []byte
    sl := make([]string, 0)
    if e := codec.NewEncoderBytes(&b, bincHandle).Encode(sl); e != nil {
        panic(e)
    }
    var sl2 []string
    if e := codec.NewDecoderBytes(b, bincHandle).Decode(&sl); e != nil {
        panic(e)
    }
    fmt.Printf("%#v became %#v\n", sl, sl2)
}

Produces:

[]string{} became []string(nil)

Empty slices are not always nil..

[zond]

Also solved by #6 I think.

[ugorji]

I thought long and hard about this one. In the notes on decode.go, I had these comments.

// Note: In decoding into containers, we just use the stream to UPDATE the container.
// This means that for a struct or map, we just update matching fields or keys.
// For a slice/array, we just update the first n elements, where n is the length of the
// stream.

This means that if you try to decode an empty map stream into a map with 10 keys, the map with 10 keys is unchanged. If you try to decode an empty list stream into a slice with 10 elements, the slice with 10 elements is unchanged. If you try to decode a 2-element list stream into a slice with 10 elements, the first 2 elements of the slice are modified. If you try to decode an empty list stream into a nil slice, the nil slice is unchanged.

I did this for consistency. There are many ways to tackle this, but this simple consistent model felt most natural to me. I understand that different folks may want to tackle this differently, but one way had to be chosen. I will move some of these notes to the package docs.

[zond]

Oh, for my application the paramount property of the encoding is reflect.DeepEqual(data, Decode(Encode(data)).

Could you perhaps add tests to the repository that define your required behaviour - perhaps I can make the code fulfill both requirements?

[ugorji]

The full model is documented as:

// Note: In decoding into containers, we just use the stream to UPDATE the container.
// This means that for a struct or map, we just update matching fields or keys.
// For a slice/array, we just update the first n elements, where n is the length of the
// stream.
// However, if the encoded value is Nil in the stream, then we try to set
// to the corresponding "zero" value.
//
// Also, we must ensure that, if decoding into a nil interface{}, we set to a non-nil
// value except even if the container registers a length of 0.

To get what you want, decode into a non-nil value (e.g. decode into []byte{}, not []byte(nil)). reflect.DeepEqual should then work.

I want to think about a better model for the tests using a cleaner table-driven model, so we don't add tests organically. Please bear with me while I find some time in the next few weeks to do it. When I do, I'd work on adding some tests for this.

[zond]

I see your reasons, but I have a big nested structure in the form of a struct type variable, where the slice is nested quite deep.

Initializing this struct to let the slice be non-nil is a big code stink that I want to avoid :/

Basically, I want to use your project as a faster encoding/gob or encoding/json.

[ugorji]

Ah ... very valid point. That's a very important use-case that I don't account for. Let me think/work this through, and follow up by tomorrow.

[zond]

I am happy that you see my point, and am looking forward to your considered opinion on the matter :)

[ugorji]

The latest commit should fix this for both maps and slices. Thanks for bringing up the issue.