Skip to content

finish the app#3

Open
garak wants to merge 1 commit into
yceruto:masterfrom
garak:refactoring
Open

finish the app#3
garak wants to merge 1 commit into
yceruto:masterfrom
garak:refactoring

Conversation

@garak
Copy link
Copy Markdown
Contributor

@garak garak commented Sep 8, 2022

No description provided.

$this->createdAt = new DateTimeImmutable();
}

public function update(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

$blogPost->update(content: 'new content');

vs

$blogPost->changeContent('new content');

[SRP] It looks simple but in my opinion, is not a good thing when your logic starts growing or you have to do something specific for each value (like domain validations).

{
$blogPost = $this->useCase->execute($id, $author, $title, $content);

if ('json' === $format) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

[OCP] This is open for modification. An extension mechanism would be needed to avoid modifying this class each time a new format is added.

{
$blogPosts = $this->useCase->execute();

if ('json' === $format) {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

[OCP] same here, this is open for modification and can be avoided by creating a dedicated class or method to do this job without asking for the format here, e.g.:

$this->renderer->renderCollection($blogPosts);

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