Golang Code Review

(Anyaddres) #1

Please can someone take the time to review my work? Need some feedback. I have the application working but am not sure if I have followed best practices and idiomatic ways… https://github.com/anyaddres/supermann

(Ivan Matmati) #2

I had a rapid look over you code. There might be a problem with geoip.go. As regards the NewGeoIP, it’s deceptive. It says if you want a new GeoIP call me. But in fact it overrides the singleton instance of gip. Why did you do it this way ? Would it be inappropriate to provide a new one at each call ? Of course if you change from singleton to prototype creation, the CloseGeoIPHandle should be a struct function. Hope it could help.

(Anyaddres) #3

Hi Ivan.
First of all thank you for your response. What I wanted to do was maintain a singleton instance of the db handle when the app server comes up. However I did make a mistake which you rightly pointed. I have now modified the code both for the db handle and the geoip db handle to be singletons. https://github.com/anyaddres/supermann/blob/master/geoip/geoip.go#L20 and https://github.com/anyaddres/supermann/blob/master/geoip/geoip.go#L32.
For the db https://github.com/anyaddres/supermann/blob/master/datastore/db.go#L46

Do you see anything else that I might need to change?

(Ivan Matmati) #4

Hi, I think you can go :slight_smile: