Criticize my first script

Hello folks, I'm learning Go, coming from Python and some NodeJS, I just fell in love with how easy it is to distribute self sufficient binaries in Go and how fast it is, two features that are important at my job.

I've written a little script to monitor license usage for a software I work with, but I feel like my code is all over the place, would love if someone could review it and give me tips on how to organize it better, if I'm doing something completely wrong, etc.

I've tried to find a subreddit for people learning go but did not find one, I also bough the book "The Go Programming Language" but just started reading it.

Here is the code https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go

---

评论：

everdev:

I would use a flat file structure: https://blog.golang.org/organizing-go-code

Most other languages tend to use folder to separate functionality, but Go uses them in a more robust way to define reusable code.

https://golang.org/doc/code.html is also a good place to learn

Small functions are good, but this one doesn't really add value:
https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go#L134:L140 and can be combined with processLicenseModules

https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go#L73 processLink uses some deep if/else logic and some fmt.Printf statements. In go, it's typically better to return values back to func main() like:

func main() {
  output, err := myFunc(1)
  if err != nil {
    panic(err)
  }
  fmt.Println(output)  
}
func myFunc(i int) (string, error) {
  if i < 0 {
    return "", errors.New("i must be >= 0")
  }
  return "i is >= 0", nil
}

This way, your functions are more easily unit testable and you have a simpler API in your app. myFunc can do lots more work, like your functions do, but I know I can toss it an int and get back either a string or an error. It keeps your apps more predictable and helps you refactor down the road.

TheV295:

This is amazing feedback, thanks for taking the time!

jackmcmorrow:

!redditsilver

RedditSilverRobot:

Here's your Reddit Silver, everdev!

/u/everdev has received silver 1 time. (given by /u/jackmcmorrow) info

MarcelloHolland:

Ok, let's say you look at this code after  a year, would you still be able to "know" what it does?
I would suggest to write inline documentation, or at least some useful comments.
Some functions have been split up too much, and some need to be split up (like the "processLink" does 2 things).
And for the first attempt, I rate this an 8 :-), now go on, and make it a 10 :-)

TheV295:

Makes a lot of sense, thanks for the feedback!

soapysops:

You don't need to explicitly name the models package after importing it
It's usually better to avoid having multiple files in a package, mainly because once they are combined, it's really hard to pull them apart, and it's really easy to accidentally use a constant or variable defined in another file in the package. The Go stdlib mushes files together like this frequently, but they usually are very specific about what they put in each file. For example, the http package has a file that just has all the HTTP status codes in it. But having functions and other more complex things like that in multiple files can make life really difficult. Honestly, for your use case I probably wouldn't even put all those types into a separate package - but that's just me.
models should be named model to be more consistent with package naming conventions.
Your doc comments don't always follow the recommended format. (It's nitpicky, but I thought I'd let you know; I think the go linter would probably catch those errors).
Many Go projects that produce binaries will have a "cmd" directory containing packages with all the func main()'s and a "pkg" directory containing packages with reusable or refactored code. If you Google around you can probably find a few posts on the subject. Everybody has their own thoughts on the right way to organize Go code, but I do find that putting files with func main in separate directories can make it much easier to figure out how the codebase works. Also, I personally really don't like putting Go files in the root of a repo, but again that's personal preference and doesn't really matter for a project this small.

TheV295:

Thanks for these, very valuable info

hariador:

First criticism, it's not a script.

TheV295:

Noted, thanks, care to define it?

OlyGhost:

It's a program.