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

BC-7772 introduce ddd objects #5167

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

CeEv
Copy link
Contributor

@CeEv CeEv commented Aug 5, 2024

Description

Links to Tickets or other pull requests

Changes

Datasecurity

Deployment

New Repos, NPM pakages or vendor scripts

Approval for review

  • DEV: If api was changed - generate-client:server was executed in vue frontend and changes were tested and put in a PR with the same branch name.
  • QA: In addition to review, the code has been manually tested (if manual testing is possible)
  • All points were discussed with the ticket creator, support-team or product owner. The code upholds all quality guidelines from the PR-template.

Notice: Please remove the WIP label if the PR is ready to review, otherwise nobody will review it.

@CeEv CeEv self-assigned this Aug 5, 2024
@CeEv CeEv requested review from Metauriel and EzzatOmar August 5, 2024 08:18
describe('When value object with modification is created.', () => {
const setup = () => {
class Name extends ValueObject<string> {
protected modified(value: string): string {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Replacing the boilerplate with array of functions that can be execute will improve the usability
modifications: [this.removeWithspacesAtBeginning, this.truncatedToLength4]
validations: [TypeGuard.isString, this.isValidLength]

In this case it is also easy to use same hooks for different value objects. But it is harder to implement with ts. We should look togehter into it.

});
});

describe('By usage of primitive array as value object', () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"of array with primitive values"

// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return value;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO:

}

/** Use this method with override for add modifications, before execute the validation. */
protected modified(value: T): T {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO:

export abstract class ValueObject<T extends ValueObjectTyp> {
public readonly value: T;

constructor(value: T) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO:

@CeEv CeEv marked this pull request as ready for review September 16, 2024 09:35
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
1 Uncovered Lines on New Code (required ≤ 0)

See analysis details on SonarCloud


type ValueObjectTyp = PrimitiveType | PrimitiveTypeArray;

export abstract class ValueObject<T extends ValueObjectTyp> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ergibt es überhaupt Sinn eine abstrakte Klasse für Value Objects einzuführen? Geht uns dadurch nicht ganz viel Fachlichkeit verloren, wenn zB der Wert immer Value heißt? Was an der abstrakten Klasse bietet wirklich einen Mehrwert beim entwickeln?

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.

2 participants