search

gonum / plot

A repository for plotting and visualizing data
BSD 3-Clause "New" or "Revised" License
2.72k stars 202 forks source link

vg/recorder: invalid/unused Canvas.c field #771

Closed sbinet closed 1 year ago

sbinet commented 1 year ago

I was in the middle of implementing a JSON-based vg/recorder.Canvas (b/c of https://github.com/go-hep/hep/issues/982), looking at the code in vg/recorder.

when I noticed:

func (c *Canvas) append(a Action) {
    if c.c != nil {
        a.ApplyTo(c)
    }
    c.Actions = append(c.Actions, a)
}

I think a.ApplyTo(c) should read a.ApplyTo(c.c). right ?

but Canvas.c is never accessed (outside of Canvas.append, that is), and cannot be modified outside of vg/recorder. so, couldn't we just get rid of this field ?

kortschak commented 1 year ago

On Fri, 2023-09-01 at 06:30 -0700, Sebastien Binet wrote:

I was in the middle of implementing a JSON-based vg/recorder.Canvas (b/c of go-hep/hep#982), looking at the code in vg/recorder. when I noticed: func (c *Canvas) append(a Action) { if c.c != nil { a.ApplyTo(c) } c.Actions = append(c.Actions, a) } I think a.ApplyTo(c) should read a.ApplyTo(c.c). right ? but Canvas.c is never accessed (outside of Canvas.append, that is), and cannot be modified outside of vg/recorder. so, couldn't we just get rid of this field ?

I think you are correct on both of these. I don't remember what I was thinking when that was written. My suspicion is that I was thinking about replay, but it's not required for that either.

Dan