<p>It's super simple: It queuries alphavantage for given stocks, and exposes it on a REST api.</p>
<p>It uses a JWT from Firebase users linked to the project to validate queuries, and it can be automatically deployed to Google Cloud App Engine. It does a lot of things in that small amount of code, I think.</p>
<p>It's my first "real" Go project that actually has real functionality, but I am certain there are things in there that make your toes cringe. I'm very open for criticism as on how to improve. Even basic stuff such as project structure(even though it's small, of course)</p>
<p><a href="https://github.com/cbll/stockmarket-service-go" rel="nofollow">https://github.com/cbll/stockmarket-service-go</a></p>
<p>Thanks in advance! Hoping I will be able to move from Java/Javascript into a Go-position sometime in my career. It feels wizardy in a nice way to code in. Also, dat compile time.</p>
<hr/>**评论：**<br/><br/>whizack: <pre><p>your lib module name needs to be something actually descriptive. it also handles multiple disparate things that aren't entirely related. separate those concerns properly.</p>
<p>the url you contact should probably be multiple environment variables (one for the format string, the other for API key). You'll need this even more if you actually test your service with fakes for failure scenarios. ensure they have sane default values.</p>
<p>the stock symbols are another one. make these configuration values. If I wanted to add a different stock I (nor you) should need to recompile.</p>
<p>lib.MarketDataMap screams poor abstraction to me. Modules aren't objects or containers, don't treat them like one because it makes them harder to test.</p>
<p>speaking of tests, you have zero tests but a lot of functionality that has dependencies. abstract those components so you can test them properly.</p>
<p>when your stock list is small it's probably fine, but your tight loop to check all of them at once will probably get throttled (if they do any throttling that is) so you should expect it rather than trust they'll always let you spam them with requests. Thinking about the list of stocks as a queue and pulling them on an interval so you're not spamming the service that provides your data is a 'good citizen' thing to do.</p>
<p>time.AfterFunc() recursive chains is a node.js (read: not idiomatic) way to handle concurrent routines. Don't do this because it's a lot harder to debug failures. Prefer an infinite for {} with proper shutdown/cancel signaling. You should also be handling shutdown/interrupt/cancel signals in general.</p></pre>cbll: <pre><blockquote>
<p>your lib module name needs to be something actually descriptive. it also handles multiple disparate things that aren't entirely related. separate those concerns properly.</p>
</blockquote>
<p>So, split them up into "security" and .... "data" or something?</p>
<blockquote>
<p>the url you contact should probably be multiple environment variables (one for the format string, the other for API key). You'll need this even more if you actually test your service with fakes for failure scenarios. ensure they have sane default values.</p>
</blockquote>
<p>What is best practice for this? Read it from a .yml file?</p>
<blockquote>
<p>the stock symbols are another one. make these configuration values. If I wanted to add a different stock I (nor you) should need to recompile.</p>
</blockquote>
<p>This, for sure, I have thought about! Even passing them as an argument when starting up the service, or something similar.</p>
<blockquote>
<p>lib.MarketDataMap screams poor abstraction to me. Modules aren't objects or containers, don't treat them like one because it makes them harder to test.</p>
</blockquote>
<p>Can you elaborate on this - I don't think I quite understand.</p>
<blockquote>
<p>time.AfterFunc() recursive chains is a node.js (read: not idiomatic) way to handle concurrent routines. Don't do this because it's a lot harder to debug failures. Prefer an infinite for {} with proper shutdown/cancel signaling. You should also be handling shutdown/interrupt/cancel signals in general.</p>
</blockquote>
<p>So, instead of afterFunc, I should just use an infinite for loop which calls the function itself recursively at a specified interval?</p>
<p>And yeah I'm coming from Java but have used Python and Node a bit too, so I have some brain damage for sure :P</p>
<p>Thanks for taking the time to look at it!</p></pre>halfstar: <pre><p>An empty for {} will run until you specify a return, or you can listen for a signal from the OS to interrupt. Example here - <a href="https://gist.github.com/reiki4040/be3705f307d3cd136e85" rel="nofollow">https://gist.github.com/reiki4040/be3705f307d3cd136e85</a> </p>
<p>For example:</p>
<pre><code>package main
import (
"fmt"
)
func main() {
i := 0
for {
i++
fmt.Println(i)
if i > 10 {
fmt.Println("i is now greater than 10, returning")
return
}
}
}
</code></pre></pre>cbll: <pre><p>But how would you add recursion as I did, for example?</p></pre>halfstar: <pre><p>Recursion is unnecessary, a more idiomatic go approach would be to run the infinite forloop in a goroutine and share the data through a channel.</p></pre>cbll: <pre><p>And then set an interval of which to call the API elsewhere? Since I can't spam it down.</p></pre>halfstar: <pre><p>You can use the time sleep function to wait during the for loop in the go routine </p></pre>cbll: <pre><p>Haven't used channels yet but I will look it up. Cheers :) </p>
<p>For the for loop, it'll be something like..?</p>
<pre><code>for {
// go <function>
// sleep function?
}
</code></pre>
<p>And it would call the function, sleep for N seconds, then call the function, sleep etc. indefinitely? </p></pre>
