aliuygur / golan-clean-code-example

a real world example web app in Go

Golang clean code example

This project is just an real world example to understand and tell these in GO;

• How to organize folders
• How to test go project
• Dependency management
• Error handling
• RESTful APIs
• Serving in Docker

To solve the some above problems have been used these Approaches;

• Uncle Bob's "The Clean Architecture"
• Andrew Gerrand's, "Error handling and Go"
• SOLID design principles

Sources

• https://8thlight.com/blog/uncle-bob/2012/08/13/the-clean-architecture.html
• http://manuel.kiessling.net/2012/09/28/applying-the-clean-architecture-to-go-applications/
• https://github.com/jaijiv/handybid
• https://medium.com/@benbjohnson/standard-package-layout-7cdbc8391fc1
• https://talks.golang.org/2014/organizeio.slide#1
• http://butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod
• https://scotch.io/bar-talk/s-o-l-i-d-the-first-five-principles-of-object-oriented-design
• https://blog.golang.org/error-handling-and-go

golan-clean-code-example's Issues

overridding Gopath for custom build

I did not want to submit a PR because you may be ideologically opposed to my suggestions. I think it is good to provide good example code. I feel there are some big tweaks that should happen in this example service. This is a brief code review and I did not dig in too deep. There could be more application level issues. I just took note of the things I noticed relatively quickly.

This application is not go get-able and your shell scripts override the GOPATH. While it might "work," I would not consider this idiomatic Go code. Let developers manage their own GOPATH and let other get your packages with go get.

You have an odd import path here: https://github.com/alioygur/gocart/blob/master/src/app/usecases/facebook.go#L11. The lack of a namespace from github or where ever could lead a newer-to-Go developer to think that it is part of the standard lib. I think this is a fragment of you taking over the GOPATH.

Consider changing this to move all of the src/app code to the root directory and creating a bin/ directory where the majority of your of you current root files (the executable ones at least) live. Where I work, we have scripts like bin/build and bin/test that our build systems leverage. These can have build tags and standard command line arguments like making sure tests use the -race flag.

You have no tests :(. I'm interested in the testing path you would take. You could leverage httptest. You could have integration style tests that test integration with the DB (I usually find mocking that to be not worth it).

You have IDatabase as an interface. It is more of a Java paradigm to preface with i. Usually, Go interfaces have a suffix or -er or -or which you use earlier in that same file.

You have Amount float32 - you should use int for money to avoid unexpected rounding issues.

Named returns typically hurt readability: https://github.com/alioygur/gocart/blob/master/src/app/order.go#L102

JWTs should be short lived as you can't invalidate them. If something were to be exploited, you would be allowing that exploited token to do what ever they wish for a whole week: https://github.com/alioygur/gocart/blob/master/src/app/user.go#L51

Your parseDBURL is scary because of the url parsing suggests that someone could accidentally use it in a place that would be exposed to an end user's tampering. I see that it actually comes from an Env Var. Why not just have the Domain Name String as the Env Var? Or have separate DB Env Vars that you read (user, pw, etc)?

Here (https://github.com/alioygur/gocart/blob/master/src/app/infra/cloudinary.go#L64) and in other locations, you are not checking for errors (many foo, _ := ...). The linked one is particularly insidious as someone not familiar with that method would not realize that an error was not checked. I believe there is a package called errCheck that would catch things like that. I think GoMetaLinter runs that be default.

You override the global rand seed here: https://github.com/alioygur/gocart/blob/master/src/app/infra/utils.go#L9. If someone were to import that code, they would lose control of the rand seed which they may be using for deterministic results (which is something I would do, for instance, in a test suit with a -seed flag). Instead, you should instantiate a new rand value with its own seed.

In your gorm interface directory, you have interface{} everywhere, losing any sense of type safety. Looks to be the same in the bolt files. interface{} tells the developer (and the compiler) nothing.

Consider avoiding raw ints for status code responses as you do here: https://github.com/alioygur/gocart/blob/master/src/app/interfaces/handlers/account.go#L35. Consider http.StatusOK.

What does https://github.com/alioygur/gocart/blob/master/src/app/interfaces/handlers/auth.go#L121 get you? Why not just have a token struct? Then you don't have the interface{} return value that tells a developer nothing.

Consider changing decodeR to decodeReq or some similar name expansion. After jumping around looking at the usage and the definition, I see what it does. It would nice if the name was less cryptic with the R.

You check for user existence with userExistByEmail; unfortunately that is a bit naive. If your service gets popular, you will be faced with the use case where a husband and wife (or similar) share an email account but do not want to share an account on your system. Crazy? Yes. Happens? Yes.

Why the label here? https://github.com/alioygur/gocart/blob/master/src/app/usecases/facebook.go#L36

I don't see you closing this Get request: https://github.com/alioygur/gocart/blob/master/src/app/usecases/facebook.go#L46. This will lead to leaked file descriptors and eventually the inability to make connections.

You use an ORM - to each their own. I do not care for them. I particularly don't care for GORM as it takes a code-first perspective, when, in production systems, you usually need to take a database first perspective.

I don't see any consistent logging.

There is no instrumentation / metrics.

I hope to only provide some constructive criticism. Cheers and good luck.