ohler55 / ojg

Optimized JSON for Go
MIT License
857 stars 49 forks source link

Builder.Pop doesn't work as I would expect #95

Closed nono closed 2 years ago

nono commented 2 years ago

When using the Pop() method of an alt.Builder on nested objects, I would expect to close the last object and move one level up. But it doesn't seem to be the case.

Code is probably better to explain that. I have modified a test:

func TestBuilderObject(t *testing.T) {
    var b alt.Builder

    err := b.Object()
    tt.Nil(t, err, "b.Object()")
    b.Pop()
    v := b.Result()
    tt.Equal(t, map[string]interface{}{}, v)

    b.Reset()
    tt.Nil(t, b.Result(), "b.Result() after reset")

    err = b.Object()
    tt.Nil(t, err, "first b.Object()")
    err = b.Value(true, "a")
    tt.Nil(t, err, "b.Value(true, a)")

    err = b.Object("b")
    tt.Nil(t, err, "second b.Object()")
    err = b.Value(false, "c")
    tt.Nil(t, err, "b.Value(false, c)")

    b.Pop()
    err = b.Value(nil, "d")
    tt.Nil(t, err, "b.Value(nil, d)")
    b.PopAll()

    v = b.Result()
    tt.Equal(t, map[string]interface{}{"a": true, "b": map[string]interface{}{"c": false}, "d": nil}, v)
}

And running the test gives:

$ go test -run TestBuilderObject
--- FAIL: TestBuilderObject (0.00s)
    util.go:31:
        expect: (map[string]interface {}) map[a:true b:map[c:false] d:<nil>]
        actual: (map[string]interface {}) map[a:true b:map[c:false d:<nil>]]
        github.com/ohler55/ojg/tt.Equal @ /home/nono/dev/ojg/tt/equal.go:26
        github.com/ohler55/ojg/alt_test.TestBuilderObject @ /home/nono/dev/ojg/alt/builder_test.go:67

Is it a bug or the expected behavior?

ohler55 commented 2 years ago

Looks like a bug to me. I'll get that fixed. I'm travelling but should be able to get to it tonight.

ohler55 commented 2 years ago

Please try the object-pop branch.

nono commented 2 years ago

It has been fixed by #97