Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Introduce domain package #21

Merged
merged 17 commits into from
Apr 18, 2020
Merged

feat: Introduce domain package #21

merged 17 commits into from
Apr 18, 2020

Conversation

bxcodec
Copy link
Owner

@bxcodec bxcodec commented Dec 23, 2018

Hello, everyone who reads this and (maybe who also used my proposed architecture in Go).

Thank you very much for the feedback that I received so far. Just to be honest, I'm very new here in the architecting software worlds. So, I'd rather call it folder structure rather than architecture. As we know, software architecture is not just a single application, but a whole business that architected into one or many applications.

Actually, for the current version in the master branch (when I made this PR), nothing wrongs. By far, this project structure solved many cases of my projects.

But, in this recent months, I try several improvements (also with looking at other people's architecture in Go), so now I decide to introduce a domain package.

In my current structure, we will find something like this:

models
├── article.go
├── author.go
└── errors.go 

article
├── delivery
│   └── http
│       ├── article_handler.go
│       └── article_test.go
├── mocks
│   ├── ArticleRepository.go
│   └── ArticleUsecase.go
├── repository //Encapsulated Implementation of Repository Interface
│   ├── mysql_article.go
│   └── mysqlarticle_test.go
├── repository.go // Repository Interface
├── usecase //Encapsulated Implementation of Usecase Interface
│   ├── articleucase_test.go
│   └── artilce_ucase.go
└── usecase.go // Usecase Interface.

So there are will be many packaged module like author, article that contains the implementation and also the contract ArticleUsecase, ArticleRepository, AuthorRepository

So, just out of curiosity, I tried a new improvement that proposed by Ben Johnson here: https://medium.com/@benbjohnson/standard-package-layout-7cdbc8391fc1
the domain package. But instead of to move it into the root project, I'd rather move it into a single domain, just for the sake consistency with my previous layout that using package models

So in my previous layout, I used models and now I renamed it to domain then move all the interface contract (Usecase and Repository) into this domain package.

So it will be more like this:

domain
├── mocks
│   ├── ArticleRepository.go
│   ├── AuthorRepository.go
│   └── ArticleUsecase.go
├── article.go
├── author.go
└── errors.go 

article
├── delivery
│   └── http
│       ├── article_handler.go
│       └── article_test.go
├── repository //Encapsulated Implementation of Repository Interface
│   └── mysql
│       ├── mysql_article.go
│       └── mysqlarticle_test.go
└── usecase //Encapsulated Implementation of Usecase Interface
    ├── articleucase_test.go
    └── artilce_ucase.go

I don't know yet, is this new layout better than the current layout I used. But, I'll try to use this new layout for my projects. If anything happens, then, this PR will be closed. But if it's good and more comfortable for the developer to use it, then I'll merge this to the branch master. :D


Anyway, if you're a Golang Engineer too, I'd like to hear your opinion about this new proposed layout :D

@sananguliyev
Copy link

sananguliyev commented Feb 26, 2019

I think this structure is better in terms of the DDD.

@jcorry
Copy link

jcorry commented Apr 6, 2019

I'd love to see discussion on this from Go architects more experienced than myself.

Copy link

@zhuharev zhuharev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link

@Anamican Anamican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks much cleaner

for rows.Next() {
t := new(models.Article)
t := domain.Article{}

This comment was marked as off-topic.

@@ -62,14 +61,14 @@ func (m *mysqlArticleRepository) fetch(ctx context.Context, query string, args .
return result, nil
}

func (m *mysqlArticleRepository) Fetch(ctx context.Context, cursor string, num int64) ([]*models.Article, string, error) {
func (m *mysqlArticleRepository) Fetch(ctx context.Context, cursor string, num int64) ([]domain.Article, string, error) {

This comment was marked as resolved.

@@ -82,44 +81,43 @@ func (m *mysqlArticleRepository) Fetch(ctx context.Context, cursor string, num i
return res, nextCursor, err

}
func (m *mysqlArticleRepository) GetByID(ctx context.Context, id int64) (*models.Article, error) {
func (m *mysqlArticleRepository) GetByID(ctx context.Context, id int64) (res domain.Article, err error) {

This comment was marked as resolved.

}

return a, nil
return

This comment was marked as resolved.

}
return a, nil
return

This comment was marked as resolved.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jojoarianto actually @bxcodec using named return, so at the end of function, you need to put return
and for pointer afaik is not good using to many pointer, because you will using heap memory instead of stack and will allocated more

ID: 1,
Name: "Iman Tumorang",
}
mockAuthorrepo := new(_authorMock.Repository)
mockAuthorrepo := new(mocks.AuthorRepository)

This comment was marked as off-topic.

@@ -152,18 +151,18 @@ func TestStore(t *testing.T) {
}

func TestDelete(t *testing.T) {
mockArticleRepo := new(mocks.Repository)
mockArticle := models.Article{
mockArticleRepo := new(mocks.ArticleRepository)

This comment was marked as resolved.

@@ -0,0 +1,36 @@
package domain

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ask: Why not domains (plural)?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please NOT message in my comments and message directly to the author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments here make good read for new people like us, to understand why this way and not the other way. also pull requests are for code review and comments right?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with that, but thats not my point.
If you scroll down to the end of this page, there is a comment box, what he has done is replied to my comment similar to what you are doing

Copy link

@PriteshJain PriteshJain May 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh I see, missed that in a long page scroll. Very sorry

@bxcodec
Copy link
Owner Author

bxcodec commented Apr 30, 2019

Hi @jojoarianto,

In this PR there are many changes related to pointer just for safer in memory allocation.
There is an explanation about this here https://segment.com/blog/allocation-efficiency-in-high-performance-go-services/

So to summarize, I think, if we really don't need a pointer, we better to avoid it.
It's just a part of the optimization in Golang. If you're not really concern about memory allocation, it's okay to ignore this.

@bxcodec bxcodec mentioned this pull request Sep 5, 2019
@AlexGrs
Copy link

AlexGrs commented Apr 15, 2020

hey ! Curious to know if you did continue with this domain approach or if you revert back to using the previous one (models at the root) with use cases in each sub-packages.

@bxcodec
Copy link
Owner Author

bxcodec commented Apr 18, 2020

Hi @AlexGrs , for microservice I use this.

But for the monolith, I use the previous one, because Monolith will look messier if we organized like this.
I mean the contracts all in one place

@bxcodec bxcodec merged commit d452858 into master Apr 18, 2020
@bxcodec bxcodec deleted the introduceDomainPackage branch April 18, 2020 06:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants