My first Go program -- Any constructive criticism?

<p><a href="https://github.com/cascwo/go-garlic" rel="nofollow">https://github.com/cascwo/go-garlic</a></p> <p>Some of you may have heard about <a href="https://garlicoin.io/" rel="nofollow">garlicoin</a>, a new memecoin launched a few days ago. Two of the most popular wallet choices are the default cli-wallet, and garlium. One of my favorite features of garlium is that it sends a notification whenever you get paid. I use the cli-wallet and wanted this feature, so here is my attempt at it. </p> <p>Some points:</p> <ul> <li><p>My main issue is with the toast library, it seems that a windows update ruined the ability to use any appid, so I have a hacky workaround where I use the powershell appid instead. </p></li> <li><p>I need to make a config file where users can put their wallet address, but I don&#39;t know the best way to do this. Just use a text file and read the first line?</p></li> <li><p>for the notification image, I need to use an absolute path to the image. Is there a way to change the code so a user doesn&#39;t need to edit this path?</p></li> </ul> <p>Thanks to anyone who takes a look and tells me how I can write Go code in a more idiomatic and effective way!</p> <hr/>**评论：**<br/><br/>sh41: <pre><p>I have one suggestion I can make.</p> <p>In Go, it’s common to order code from higher level functions first to lower level helpers second. That way, when reading code, you first encounter higher level, easier to understand code that helps provide context for the helpers. Then, once you&#39;re familiar with how a helper is being used and why it’s needed, reading its code is easier.</p> <p>So, I’d suggest putting main on top, and the two helpers, in the order they’re used, below.</p></pre>nesigma: <pre><blockquote> <p>In Go, it’s common to order code from higher level functions first to lower level helpers second.</p> </blockquote> <p>Is this really the case? Are there any good examples from the standard library or maybe a code review guide on this subject?</p></pre>sh41: <pre><p>Unfortunately, there isn’t an entry for this at <a href="https://golang.org/s/style" rel="nofollow">https://golang.org/s/style</a> or similar documents (I might want to put together an entry at <a href="https://dmitri.shuralyov.com/idiomatic-go" rel="nofollow">https://dmitri.shuralyov.com/idiomatic-go</a> meanwhile so I can link to it), but I see this done very commonly in standard library and other idiomatic Go code. I always appreciate it because it makes reading unfamiliar code much easier.</p> <p>For example, look at <a href="https://github.com/golang/go/blob/master/src/encoding/json/decode.go" rel="nofollow">https://github.com/golang/go/blob/master/src/encoding/json/decode.go</a>. See how it starts out having the highest level, exported symbols first, and the lower level nitty and gritty implementation details follow below.</p> <p>This isn’t done in some languages where top level symbols must be declared before you can use them. In Go, that implementation restriction isn’t present—you can define package scope identifiers in any order even if they reference each other—so it’s great to make use of it to make code more readable.</p></pre>Redundancy_: <pre><p>I believe that it&#39;s from Clean Code by &#34;Uncle&#34; Bob Martin.</p></pre>cascwo: <pre><p>ah okay. I am used to having main at the bottom of the file, just habits! Thanks for the info.</p></pre>natbobc: <pre><p>Congrats!</p> <p>Consider adding comments to your functions. It&#39;s pretty small now so probably don&#39;t need to worry about splitting it into different packages. If it grows then I&#39;d consider splitting it into a library and command package.</p> <p>I would put this as a defer after L17; <a href="https://github.com/cascwo/go-garlic/blob/master/main.go#L19" rel="nofollow">https://github.com/cascwo/go-garlic/blob/master/main.go#L19</a></p> <p>I would probably turn the wallet ID into a flag (<a href="https://godoc.org/flag" rel="nofollow">https://godoc.org/flag</a>); <a href="https://github.com/cascwo/go-garlic/blob/master/main.go#L42" rel="nofollow">https://github.com/cascwo/go-garlic/blob/master/main.go#L42</a></p> <p>I would probably embed this in the app and unpack it on start-up to $HOME/.yourapp/garlic.png or somewhere similar using statik or go-bindata; <a href="https://github.com/cascwo/go-garlic/blob/master/main.go#L33" rel="nofollow">https://github.com/cascwo/go-garlic/blob/master/main.go#L33</a></p> <p>I would probably change the function return signature to (float64, error). It might be recoverable depending on the error so Fatal will result in shutting down the app when it might not be necessary. <a href="https://github.com/cascwo/go-garlic/blob/master/main.go#L13" rel="nofollow">https://github.com/cascwo/go-garlic/blob/master/main.go#L13</a></p></pre>cascwo: <pre><p>First of all, thanks a ton for the reply! Lots of useful info!</p> <p>Yes comments should be added definitely.</p> <p>What exactly do you mean by splitting it into a library and command package? Which parts of my code would go into those packages respectively?</p> <p>I did not know about defers but after looking them up that definitely seems like the way to do it.</p> <p>Flags are an option but my plans were to just launch an .exe by double clicking, not using the command line at all, so flags wouldn&#39;t be correct choice right?</p> <p>I will look into statik and go-bindata. If I can get it to work I think that is a great solution.</p> <p>I don&#39;t exactly know how I would recover the app if the user gave an invalid address, my thoughts were to shut down the app, fix the invalid address, and relaunch. </p> <p>Thanks again for the help. I just started learning go and have only spent a few hours with it, but I think it is really great!</p></pre>natbobc: <pre><p>re: library and package see this post by Dave; <a href="https://dave.cheney.net/2014/12/01/five-suggestions-for-setting-up-a-go-project" rel="nofollow">https://dave.cheney.net/2014/12/01/five-suggestions-for-setting-up-a-go-project</a></p> <p>re: flags; I would probably create a config file so users wouldn&#39;t need to recompile the app when changing the address. If it&#39;s personal use only and you don&#39;t plan to distribute it to others then I&#39;d leave it as is.</p> <p>re: statik; if you&#39;re not targeting general distribution it&#39;s fine without but it&#39;s generally good practise to not use hardcoded paths that are username/os specific.</p> <p>re: recovery; in that scenario you could do a couple of things; 1 tell the user they&#39;ve entered an invalid address and prompt them to enter a new one. 2. tell the user they&#39;ve entered an invalid address as you&#39;ve suggested. For both you would only do that for the first request prior to entering the loop. If it fails exit/prompt. If it fails inside the loop you could add a counter that is reset with every success and increments for each failure. After a certain threshold you could either exit or exponentially back off/jitter trying less frequently with each failure thereby allowing the upstream system some breathing room to recover. <a href="https://aws.amazon.com/blogs/architecture/exponential-backoff-and-jitter/" rel="nofollow">https://aws.amazon.com/blogs/architecture/exponential-backoff-and-jitter/</a></p> <p>Cheers!</p></pre>cascwo: <pre><p>I really appreciate all the help it man</p></pre>BostonArtExchange: <pre><p>My recommendations would be:</p> <ol> <li><p>On line 23 of main.go, if you are not going to check the error then just assign it to _ instead of err. Not a big deal but IMO this is a little cleaner.</p></li> <li><p>Check the response code of the http request for getting balance, and handle errors accordingly. You may not necessarily want to always exit the program if an http request fails.</p></li> <li><p>You might want to retrieve the relative path for the icon rather than absolute, that way it works on any computer and not only yours. It also would be good practice to store the hard coded strings as constants declared at the top of the file. That&#39;s just a stylistic issue though.</p></li> <li><p>&#34;difference&#34; doesn&#39;t need to be declared on line 45 of main.go</p></li> </ol> <p>Other than that it looks pretty good. Nice job!</p></pre>zoricj: <pre><p>Also, on line 53, it would be more readable like time.Sleep(10 * time.Second) </p></pre>: <pre><p>[removed]</p></pre>garlicbot: <pre><p><a href="https://i.imgur.com/etMqixE.jpg" title="Reddit Garlic" rel="nofollow"><strong>Here&#39;s your Reddit Garlic, cascwo!</strong></a> </p> <p><a href="/u/cascwo" rel="nofollow">/u/cascwo</a> has received garlic 1 time. (given by <a href="/u/pythonETH" rel="nofollow">/u/pythonETH</a>) </p> <p>^I&#39;m ^{^a} ^{^{^bot}} ^{^{^{^for}}} ^{^{^{^questions}}} ^{^{^{^{^contact}}}} ^{^{^{^{^{<a href="/u/flying_wotsit" rel="nofollow">/u/flying_wotsit</a>}}}}}</p></pre>