-
Notifications
You must be signed in to change notification settings - Fork 3
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
Stabilize DDD namespaces and centralize common objects #243
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Whathecode
changed the title
Refactor: split
Stabilize DDD namespaces
Mar 28, 2021
common
module types into DDD namespaces.
Whathecode
changed the title
Stabilize DDD namespaces
Stabilize DDD namespaces and centralize common objects
Mar 28, 2021
Whathecode
force-pushed
the
ddd-namespaces
branch
from
April 14, 2021 16:28
e3c61ce
to
b7a8974
Compare
As part of this, I also removed the now superfluous `tasks`, `devices`, and `triggers` subfolders in the protocols subsystem and merged them under one `configuration` folder.
This also means we no longer need separate serializers per subsystem. All subystems now use `createDefaultJSON`. And, I added a `createTestJSON` which also registeres the stub types.
Whathecode
force-pushed
the
ddd-namespaces
branch
from
April 15, 2021 17:57
b7a8974
to
acc01b9
Compare
Type inference for serializers is no longer experimental, so the default kotlinx.serialization API is clean enough.
ltj
approved these changes
Apr 19, 2021
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much cleaner. Good job
This was referenced May 15, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
According to DDD the semantics of namespaces should be:
application
: classes which are exposed to consuming clientsdomain
: internal domain logic, not exposed to clientsinfrastructure
: implementations specific to one particular infrastructure (e.g., database or serialization technology)The current codebase did not follow this strictly. There are many classes which live in the
domain
namespace yet are exposed to clients. We want to have a strict separation so that people who want to develop clients on other tech stacks know which classes need to be implemented by them (theapplication
namespace). Or, in case we do some type of code generation we know for which classes code needs to be generated.In addition, many of these classes (erroneously residing in the
domain
namespace) are reused across multiple subsystems. This is for historic reasons mostly, since the implementation of CARP started with theprotocols
subsystem. However, we have since discovered most of these classes (such asDeviceDescriptor
) are in fact more of a shared kernel (a DDD concept) and are therefore best moved to thecommon
module in aapplication
namespace. The goal should be that only thecommon
module has base types which are extensible and passed across the architecture (protocols
now).While making such big namespace changes we should probably also look at:
protocols
deployment
studies
InputElement
to theelement
namespace. It seems inconsistentData
and the concrete types live in the same namespace, but this is not the case for input elements.ApplicationService
andEventBus
) to aservices
namespace. It seems disorganized that these live side-by-side with common types such asDateTime
right now.StudyProtocol.Id
tocarp.common
.carp.protocols.domain
. (Moveddevices
,triggers
, andtasks
namespace to onecarp.protocols.domain.configuration
namespace since they only contained configuration classes forStudyProtocol
.)SerializersModule
as all these types should be moved to common.StudyProtocol
andStudyProtocolSnapshot
remains inprotocols
and best belongs there for now.)carp-protocols.md
contains mostlycarp.common
stuff.The suggested outline for new namespaces. Changes (and intended changes) in this PR are shown in bold.
tasks
,triggers
, anddevices
since most of its contents were moved tocommon
)