Conversation
|
|
||
| type CreateOrUpdatePortStore interface { | ||
| CreateOrUpdatePort(ctx context.Context, port *store.Port) error | ||
| CreateOrUpdatePort(ctx context.Context, port *repository.Port) error |
There was a problem hiding this comment.
I don' understand why the command package uses a repository model? This by pass completely the core so tightly couple two packages outside core that should be swappable...
Assume that repository.Port is specific of a DB technology (sqlite, mongo, gRPC, etc) in fact this would be a good case for subpackages (/internal/repository/port/postgres/db.go if you allow for many repositories and for many implementations), so the command would need to change if repository change. To avoid this we always have driver and driven code (left and right) of the hexagon to depend on core models.
There was a problem hiding this comment.
yeah, that's what happens when you work on multiple codebases at once :) and show something not yet completed. I'll fix it, cheers
| @@ -1,28 +1,27 @@ | |||
| package adapters_test | |||
| package repository_test | |||
There was a problem hiding this comment.
Why did you choose to have external test packages to the code package? Is this necessary?
There was a problem hiding this comment.
I planned it to be an integration test, then changed for a unit one and forgot to change, thank you.
| "github.com/evt/port-api/internal/app/command" | ||
| "github.com/evt/port-api/internal/app/query" | ||
| "github.com/evt/port-api/internal/common/server" | ||
| "github.com/evt/port-api/internal/repository" |
There was a problem hiding this comment.
Similarly to my previous comment you are tightly coupling package transport to repository, this bypass completely the core, which nullify a large benefit of hexagonal architecture
There was a problem hiding this comment.
fixed by moving sentinel errors to domain package, cheers
| } | ||
|
|
||
| func PortToDomain(p *Port) (*domain.Port, error) { | ||
| func portStoreToDomain(p *Port) (*domain.Port, error) { |
There was a problem hiding this comment.
you must check that p is not nil, which has the potential of panic for nil pointer exception.
There was a problem hiding this comment.
right, thank you. Fixed + added unit tests.
2 participants
closes: #2