Hi everyone,
I am looking for a code review to help find where my gaps in knowledge are currently.
I would like to (someday) get a job developing with Go and want to make sure the code I have hosted on GitHub is not an eyesore to look upon.
Any feedback is appreciated as I want to get better and be able to show the knowledge I have with the Go programming language.
My repo can be found at:
Thanks!
Thanks for all the tips!
I was under the impression that using i := 0 would default to an int64 since my system is 64 bit (not sure where I heard that) and wanted to keep the int as small as possible. If that is not the case then yeah, calling var is too verbose.
Awe yes! Sprintf totally overlooked that! hahaha
Thanks and I will look into DWM the DWM auto start patch. I have been running patch-less for a while to see what I need/miss from i3wm.
Good point, forgot about that.
I changed the code to use Go Routines (for example purposes) and no longer need the int in main. I want to pass the pointer through the channel but that was giving me issues so I’ll have to read up on that in tho morning.
Thanks again 
Hi jsrwab,
I didn’t have time to get into the details of your implementation, so my feedback is more a feeling than anything else. I think you should use differently your channels. In main.go, you should use a for select block. It would be nicer and more standard. The loop in wttr.go is puzzinlig me. Why not use a Ticker instead of this complex management of i ?
Don’t forget that your defer will be executed at the return of your function ! In case of a loop, it’s never executed. You’ve got a potential leak : https://golang.org/ref/spec#Defer_statements
Have a nice day.
Thanks I’ll look into all this after work and see if I can clean it up.
A small observation, is no need to explicit initialize variables to their zero values in Go.
I moved to a ticker.
I also removed the defer and just have it close the https request. In doing this will it reopen the next time the go routine for that lib is called or did I just block any updates? (Luckily the temperature does not change that often anyway lol)
Hi,
Your goroutines launches every five seconds and every hour could benefit from ticker too.
In fact, these tickers seems to be movable into the goroutines themselves. In the new versions you wrote without the loop, now the defer would be perfect because some errors could happen or premature return … You’ve got two choices :
The resp.Body.Close is actually closing the response not the request. And yes, not closing must have some bad effects somewhere as it is required from the documentation of Body field : It is the caller’s responsibility to close Body. If you don’t close the stream of the response it could mean that the connection is hold opened. Then at a certain moment, you will run out of available connections because of resource starving.
Good to know! I’ll do some more playing around and see what I can come up with.
Thanks again for all the help! Having you all look at this code is allowing me to learn more of the ins and outs Go.
By chance do you mean to defer the closing of the HTTP request as follows instead of manually closing it after the channel passes the data?
func Local(cWttr chan string) {
// get temp(%t) and wind direction/speed (%w)
// for exact location add postal code - wttr.in/~15222?format...
// for more wttr options see https://wttr.in/:help
resp, err := http.Get("https://wttr.in/?format=%t+%w")
if err != nil {
errMessage := "wttr connection issue"
cWttr <- errMessage
}
defer resp.Body.Close() // close http request
// convert responce to string for return
bodyData, _ := ioutil.ReadAll(resp.Body)
weather := fmt.Sprintf("%s | ", strings.TrimSpace(string(bodyData)))
cWttr <- weather
}
Update:
I chose implement tickers into the libs running as go routines and it works like a charm.
Thanks again to everyone that added suggestions on what I could do to improve my Go example. Can’t wait to tackle another project 
PS: I am still open to suggestions on this repo so feel free to add share your ideas. It’s on both gitlab and github.
I just commit to master on Github and only put stuff there since so many people use it. All my work in progress and branches only get pushed to Gitlab.
Some tips regarding your project structure:
1 - Create using the deps. Sometimes you’ll need dependencies.
2 - Add tests, if you are familiar with golang tests you are one step ahead.
3 - Instead of use plain string channel return, you can type your own. type weather string.
5 - Use error chan and not a only one chan to data/error.
4 - Add interfaces (you must be familiar with) and work with it.
5 - Most libraries use the cmd/binname/main.go as a entry point to compile and generate a executable file.
Thank you! I’ll implement these features next.
The goal here is to prove to any future interviewer that I can program in Go. Plus practicing these parts of Go will only.help me in nhe long run.
Hi,
defer should happen as soon as possible, before an error may occur preventing your deferred code from executing. So in your example, I’ll put it right after your Get call.
After all, you could send on a close channel and get an error !
@catacombs & @heatbr I added to simple tests to my custom packages and set up two new channels for error messages (I think I want to out put any errors to a log file at some point). The new code is up loaded and in the master branch.
@imatmati That makes sense, no reason to leave something open if something else breaks.
Thanks again everyone for the help!
I’ll be implementing more of heatbr’s suggestions this weekend.