Good morning,
I’m new to Go and wrote a basic restful API but I’m not quite sure if the Go code is ok like this or if I made some bigger mistakes.
Would it be possible for someone to review the code (it isn’t a lot yet)?
All the code is in the following repository
Thanks in advance
There are some few things I recognized (Even not exactly related to Go but in general):
btw: It seems that you are two people Sometimes you are using early return sometimes long if-else’s
First of all thanks for looking over it.
Currently the logger is really just a basis and is constantly expanded. I wanted to add that possibility and the possibility to show the filename (maybe also the path) in the future so debugging gets easier.
What would be the benefit of this? Isn’t it usually only one error that gets returned to the client? Or could there be more?
Do you have an example on how I could achieve this? I’m not quite happy with the controllers yet because I already know that most of them will be structured the same. I wanted to add a _default package with default methods for GET, POST, etc. and make the controllers use that if they don’t need special functionality.
The query functions are currently being used to fetch/insert data from the database. I did not want to return a sql.Rows so I tried to handle everything in the package. Unfortunately I have no idea how I could improve that package.
The only idea I had was to use a slice of structs (in my case forum.User) as argument and just fill that, but that did not work as I expected.
I will add error logs there, thank you.
The other person unfortunately does not have that much time anymore and is now just reviewing pull requests. He’s also new to Go though.
What about the http status-codes? 
Do you have an example on how I could achieve this?
Just a quick example:
package models
func CreatePost(...) {
...
}
CreatePost encapsulates the logic. Now you can call CreatePost from your controller
This principle is called Fat model, thin controller. It is a long-term discussion if this is better or thin model, fat controller.
But from my point of view the controller handles the application logic while the model handles the business logic.
I think there are far too many packages and files for the amount of code:
➜ forum git:(373a54e) cloc .
15 text files.
15 unique files.
3 files ignored.
github.com/AlDanial/cloc v 1.72 T=0.24 s (54.3 files/s, 2102.6 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
Go 11 84 2 343
SQL 1 11 0 50
Markdown 1 6 0 7
-------------------------------------------------------------------------------
SUM: 13 101 2 400
-------------------------------------------------------------------------------
So I refactored it. I focused on structure, leaving all current functionality intact, and ended up with:
➜ forum git:(dev) cloc .
9 text files.
9 unique files.
3 files ignored.
github.com/AlDanial/cloc v 1.72 T=0.14 s (49.5 files/s, 2720.1 lines/s)
-------------------------------------------------------------------------------
Language files blank comment code
-------------------------------------------------------------------------------
Go 5 64 0 247
SQL 1 11 0 50
Markdown 1 6 0 7
-------------------------------------------------------------------------------
SUM: 7 81 0 304
-------------------------------------------------------------------------------
Other than main, the only package that I left was uuid because its concerns were separate. Other packages were integrated into main because their concerns weren’t separate or the value they added did not justify their existence. For example, removing the Postgres package required me to move a global variable into main and change 4 lines in main.
Since I was trying to not change any functionality, the logger remains (though it is no longer a package). Please use one of the many existing logging packages instead.
Hope this helps.
First of all thank you.
My initial thought with that many packages was that I could easily swap out one thing for another. For example I could switch from Postgres to MySQL and just had to replace one package and wouldn’t have to change all the API files.
I will look through every file as soon as I’m home today.
But I got one more questions: I want to implement the GET and POST functions for the other models that will be added today but the code will probably nearly be the same as for the users. How can I add those without duplicating the code for everything?
From my point of view in Go there are often used too less packages. Often this ends up in large files (In other languages one would call it god object). I mean, one should not put every single thing into a separate package. But a separation by concerns or by business domains is very helpful for structuring the application.
Why should exactly the uuid functionality be placed in an own package?
You could use type determination by runtime and then decide dynamically which fields to save. But from my experience this kind of solution leads to a dead end when you start having cases which cannot be covered by your generic solution.
So it would be wiser to just duplicate the code and keep it simple?
One other questions I have concerning nathankerrs reply: I was always taught that the logic should be separated (MVC). Is this not the case in Go or are there different ways to structure the application?
I think I responded to everything. Please tell me if I missed something
My initial thought with that many packages was that I could easily swap out one thing for another. For example I could switch from Postgres to MySQL and just had to replace one package and wouldn’t have to change all the API files.
Assuming you aren’t using database specific stuff, this can be done by telling sqlx to connect differently. This is a one line change. If you really need to support different databases and have database specific stuff, then design an interface with a test suite along with at least two different implementations (e.g., Postgres and MySQL).
But I got one more questions: I want to implement the GET and POST functions for the other models that will be added today but the code will probably nearly be the same as for the users. How can I add those without duplicating the code for everything?
If the implementations aren’t mostly the same, then there is no duplicated code. If the implementations are small or there will not be many of them, factoring out the common stuff may not be worth it.
So it would be wiser to just duplicate the code and keep it simple?
Sometimes.
One other questions I have concerning nathankerrs reply: I was always taught that the logic should be separated (MVC). Is this not the case in Go or are there different ways to structure the application?
Packages are only one way of organizing code. The same separation that existed before my refactoring exists after it. Packages only help keep things separate by allowing non-exported things to only be directly accessed by things in the package. Note that separate is different than ease of reuse.
From my point of view in Go there are often used too less packages. Often this ends up in large files (In other languages one would call it god object). I mean, one should not put every single thing into a separate package. But a separation by concerns or by business domains is very helpful for structuring the application.
In go, packages and files aren’t objects. Separation of concerns or business domains is done in code. Packages help with separation by not allowing external access to un-exported things (e.g., types, functions, fields) and creating a separate namespace for exported things. Files help with organizing code.
I agree that it is not good to have too many different things happening in a single file or package. I also think it is not good to not group related things together.
Why should exactly the uuid functionality be placed in an own package?
I left the uuid functionality in its own package because it was well separated. If I were writing this program, I would have used someone else’s uuid package, or, because there is only one function, put the implementation in main (in uuid.go).
I too am new to Go and looking for a generally accepted reference implementation for a ReST API. Does anyone know of a repo on GitHub that would suffice for this?
Thanks.
This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.