thedevsaddam / gojsonq

A simple Go package to Query over JSON/YAML/XML/CSV Data
https://github.com/thedevsaddam/gojsonq/wiki
MIT License
2.18k stars 140 forks source link

Go-critic warnings #19

Closed Kvaz1r closed 6 years ago

Kvaz1r commented 6 years ago

go-critic linter found some issues , here corresponding log:

check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\helper.go:45:2: typeS witchVar: case 0 can benefit from type switch with assignment check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\helper.go:45:2: typeS witchVar: case 1 can benefit from type switch with assignment check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\helper.go:45:2: typeS witchVar: case 2 can benefit from type switch with assignment check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\helper.go:45:2: typeS witchVar: case 3 can benefit from type switch with assignment check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\helper.go:45:2: typeS witchVar: case 4 can benefit from type switch with assignment check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\helper.go:45:2: typeS witchVar: case 5 can benefit from type switch with assignment check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\helper.go:45:2: typeS witchVar: case 6 can benefit from type switch with assignment check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\jsonq.go:271:3: range ValCopy: each iteration copies 48 bytes (consider pointers or indexing) check-package: $GOPATH\src\github.com\thedevsaddam\gojsonq\jsonq.go:45:18: typeU nparen: could simplify to [][]query

In #18 I propose fixing for them

Why replacerange with for loop? You must create an issue regarding this refactor before sending PR.

Thanks, good point. The way that I made in my PR not very accurate and should be replaced into this one as written in description to rangeValCopy.

thedevsaddam commented 6 years ago

@Kvaz1r I think the range and current for loop doing the same, see the reason:

package main

import (
    "fmt"

    "github.com/davecgh/go-spew/spew"
)

func main() {
    list := []struct {
        Name string
    }{
        {
            "Tom",
        },
        {
            "Jane",
        },
        {
            "Joe",
        },
    }

    for _, l := range list {
        spew.Dump(&l) // copy object to the same memory address
    }

    fmt.Println("=================================")

    for i := 0; i < len(list); i++ {
        l := list[i] //  declare and initialize new variable, which also copy the object, unless you put reference (l := &list[i])
        spew.Dump(&l)
    }

    fmt.Println("=================================")

    spew.Dump(list)
}

Also the queries slice contains small amount of information

Kvaz1r commented 6 years ago

@thedevsaddam yes, I written this above:

The way that I made in my PR not very accurate and should be replaced into this one as written in description to rangeValCopy.

for i := range xs {
    x := &xs[i]
        ...
}
thedevsaddam commented 6 years ago

@Kvaz1r the for loop is not needed as the the slice will be always small. It contains only queries passed by user. I'll revert the loop to for range. I'll keep helper changes. Thank you for the PR 👍