<p>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.</p>
<p>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. </p>
<p>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.</p>
<p>Here is the code <a href="https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go" rel="nofollow">https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go</a> </p>
<hr/>**评论：**<br/><br/>everdev: <pre><p>I would use a flat file structure: <a href="https://blog.golang.org/organizing-go-code" rel="nofollow">https://blog.golang.org/organizing-go-code</a></p>
<p>Most other languages tend to use folder to separate functionality, but Go uses them in a more robust way to define reusable code.</p>
<p><a href="https://golang.org/doc/code.html" rel="nofollow">https://golang.org/doc/code.html</a> is also a good place to learn</p>
<p>Small functions are good, but this one doesn't really add value:
<a href="https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go#L134:L140" rel="nofollow">https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go#L134:L140</a> and can be combined with <code>processLicenseModules</code></p>
<p><a href="https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go#L73" rel="nofollow">https://github.com/dlopes7/appdynamics-license-monitor/blob/master/license_monitor.go#L73</a> <code>processLink</code> uses some deep if/else logic and some fmt.Printf statements. In go, it's typically better to return values back to <code>func main()</code> like:</p>
<pre><code>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
}
</code></pre>
<p>This way, your functions are more easily unit testable and you have a simpler API in your app. <code>myFunc</code> can do lots more work, like your functions do, but I know I can toss it an <code>int</code> and get back either a <code>string</code> or an <code>error</code>. It keeps your apps more predictable and helps you refactor down the road.</p></pre>TheV295: <pre><p>This is amazing feedback, thanks for taking the time!</p></pre>jackmcmorrow: <pre><p>!redditsilver</p></pre>RedditSilverRobot: <pre><h3><a href="http://i.imgur.com/x0jw93q.png" title="Reddit Silver" rel="nofollow">Here's your Reddit Silver, everdev!</a></h3>
<hr/>
<p><a href="/u/everdev" rel="nofollow">/u/everdev</a> has received silver 1 time. (given by <a href="/u/jackmcmorrow" rel="nofollow">/u/jackmcmorrow</a>) <strong><a href="http://reddit.com/r/RedditSilverRobot" rel="nofollow">info</a></strong></p></pre>MarcelloHolland: <pre><p>Ok, let's say you look at this code after a year, would you still be able to "know" what it does?</p>
<p>I would suggest to write inline documentation, or at least some useful comments.</p>
<p>Some functions have been split up too much, and some need to be split up (like the "processLink" does 2 things).</p>
<p>And for the first attempt, I rate this an 8 :-), now go on, and make it a 10 :-)</p></pre>TheV295: <pre><p>Makes a lot of sense, thanks for the feedback!</p></pre>soapysops: <pre><p>You don't need to explicitly name the models package after importing it</p>
<p>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.</p>
<p>models should be named model to be more consistent with package naming conventions.</p>
<p>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).</p>
<p>Many Go projects that produce binaries will have a "cmd" directory containing packages with all the <code>func main()</code>'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 <code>func main</code> 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.</p></pre>TheV295: <pre><p>Thanks for these, very valuable info</p></pre>hariador: <pre><p>First criticism, it's not a script.</p></pre>TheV295: <pre><p>Noted, thanks, care to define it?</p></pre>OlyGhost: <pre><p>It's a program.</p></pre>
