Skip to main content
deleted 8 characters in body
Source Link
rose
  • 325
  • 1
  • 6

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • The %q format verb will print a string in quotes. Prefer using %q instead of \"%s\".
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • Strings can be compared directly with !=, no need to call strings.Compare.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
    digest string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
}

func Digest(digest string) SaveAsFileOption {
    return func(o *opts) {
        o.digest = digest
    }
}

func SaveAsFile(url, digest string, options SaveAsFileOption...) {
    var o opts
    for _, opt := range options {
        opt(&o)
    }
    // Then you can the options through o.user, o.pass, etc.
}

Callers can call SaveAsFile either with SaveAsFile(url), or with SaveAsFile(url, Digest(digest), Auth(user, pass)). This also gives you the flexibility to define more options in the future.

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • The %q format verb will print a string in quotes. Prefer using %q instead of \"%s\".
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • Strings can be compared directly with !=, no need to call strings.Compare.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
    digest string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
}

func Digest(digest string) SaveAsFileOption {
    return func(o *opts) {
        o.digest = digest
    }
}

func SaveAsFile(url, digest string, options SaveAsFileOption...) {
    var o opts
    for _, opt := range options {
        opt(&o)
    }
    // Then you can the options through o.user, o.pass, etc.
}

Callers can call SaveAsFile either with SaveAsFile(url), or with SaveAsFile(url, Digest(digest), Auth(user, pass)). This also gives you the flexibility to define more options in the future.

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • The %q format verb will print a string in quotes. Prefer using %q instead of \"%s\".
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • Strings can be compared directly with !=, no need to call strings.Compare.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
    digest string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
}

func Digest(digest string) SaveAsFileOption {
    return func(o *opts) {
        o.digest = digest
    }
}

func SaveAsFile(url string, options SaveAsFileOption...) {
    var o opts
    for _, opt := range options {
        opt(&o)
    }
    // Then you can the options through o.user, o.pass, etc.
}

Callers can call SaveAsFile either with SaveAsFile(url), or with SaveAsFile(url, Digest(digest), Auth(user, pass)). This also gives you the flexibility to define more options in the future.

added 302 characters in body
Source Link
rose
  • 325
  • 1
  • 6

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • The %q format verb will print a string in quotes. Prefer using %q instead of \"%s\".
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • Strings can be compared directly with !=, no need to call strings.Compare.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
    digest string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
} 

func Digest(digest string) SaveAsFileOption {
    return func(o *opts) {
        o.digest = digest
    }
}

func SaveAsFile(url, digest string, options SaveAsFileOption...) {
    var o opts
    for _, opt := range options {
        opt(&o)
    }
    // Then you can access user andthe passoptions through o.user and, o.pass, etc.
}

Callers can call SaveAsFile either with SaveAsFile(url, digest), or with SaveAsFile(url, Digest(digest), Auth(user, pass)). This also gives you the flexibility to define more options in the future.

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
}
func SaveAsFile(url, digest string, options SaveAsFileOption...) {
    var o opts
    for _, opt := range options {
        opt(&o)
    }
    // Then you can access user and pass through o.user and o.pass.
}

Callers can call SaveAsFile either with SaveAsFile(url, digest), or with SaveAsFile(url, digest, Auth(user, pass)). This also gives you the flexibility to define more options in the future.

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • The %q format verb will print a string in quotes. Prefer using %q instead of \"%s\".
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • Strings can be compared directly with !=, no need to call strings.Compare.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
    digest string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
} 

func Digest(digest string) SaveAsFileOption {
    return func(o *opts) {
        o.digest = digest
    }
}

func SaveAsFile(url, digest string, options SaveAsFileOption...) {
    var o opts
    for _, opt := range options {
        opt(&o)
    }
    // Then you can the options through o.user, o.pass, etc.
}

Callers can call SaveAsFile either with SaveAsFile(url), or with SaveAsFile(url, Digest(digest), Auth(user, pass)). This also gives you the flexibility to define more options in the future.

added 9 characters in body
Source Link
rose
  • 325
  • 1
  • 6

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
}
func SaveAsFile(url, digest string, options SaveAsFileOption...) {
    var o opts
    for _, opt in:= range options {
        opt(&o)
    }
    // Then you can access user and pass through o.user and o.pass.
}

Callers can call SaveAsFile either with SaveAsFile(url, digest), or with SaveAsFile(url, digest, Auth(user, pass)). This also gives you the flexibility to define more options in the future.

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
}
func SaveAsFile(url, digest string, options SaveAsFileOption...) {
    var o opts
    for opt in options {
        opt(&o)
    }
    // Then you can access user and pass through o.user and o.pass.
}

Callers can call SaveAsFile either with SaveAsFile(url, digest), or with SaveAsFile(url, digest, Auth(user, pass)). This also gives you the flexibility to define more options in the future.

First I want to say that your code is very well written and idiomatic for someone who is new to Go.

Here are a few suggestions:

  • Prefer standard library tools for error handling rather than using an external library. errors.Wrapf can be replaced by fmt.Errorf.
  • Call file.Close with defer, so that if writing to the file fails it will still get closed. The documentation for file.Close specifies that it will only return an error if the file is already closed, so you could consider just ignoring any error from file.Close.
  • Also if writing to the temp file fails, downFile should probably delete the file to avoid leaving extra empty files.
  • This is kind of subjective, but I think downFile should return the file name instead of an *os.File because the file is already closed, so any attempt to read or write it would fail.
  • SaveAsFile has a lot of string arguments, and some of them are optional. If a caller calls SaveAsFile(url, digest, "", ""), it's not obvious that the empty strings are the username and password.

What you can use for this last point is the option pattern, I would define SaveAsFile as follows:

type opts struct {
    user string
    pass string
}

type SaveAsFileOption func(*opts)

func Auth(user, pass string) SaveAsFileOption {
    return func(o *opts) {
        o.user = user
        o.pass = pass
    }
}
func SaveAsFile(url, digest string, options SaveAsFileOption...) {
    var o opts
    for _, opt := range options {
        opt(&o)
    }
    // Then you can access user and pass through o.user and o.pass.
}

Callers can call SaveAsFile either with SaveAsFile(url, digest), or with SaveAsFile(url, digest, Auth(user, pass)). This also gives you the flexibility to define more options in the future.

added 120 characters in body
Source Link
rose
  • 325
  • 1
  • 6
Loading
Code-quote code
Source Link
Toby Speight
  • 81.8k
  • 14
  • 101
  • 309
Loading
Source Link
rose
  • 325
  • 1
  • 6
Loading