Conversation
mhsdesign
changed the base branch from
9.0
to
feature/new-link-editor-link-value-object
github-actions
bot
added
feature/new-link-editor-link-value-object
and removed
9.0
labels
| /** | ||
| * @return list<AssetId> | ||
| */ | ||
| public function getAssetIds(): array; |
There was a problem hiding this comment.
i went with an "always plural" interface as we have to account for the empty case and that a value object handles more than one asset too. So this seems the best fit.
Also we should probably specify iterable so people could yield the asset ids for simplicity during nesting.
There was a problem hiding this comment.
Iterable wound indeed be a better fit than array.
| return $this->href->getHost() . $this->href->getPath(); | ||
| } | ||
|
|
||
| public function extractNodeAggregateId(): ?NodeAggregateId |
There was a problem hiding this comment.
I'm a big fan of tryExtractWhatever if whatever is nullable, for readability
|
|
||
| use Neos\Media\Domain\Model\AssetId; | ||
|
|
||
| interface ProvidesAssetIdsInterface |
There was a problem hiding this comment.
The name sounds a bit strange why not AssetIdProviderInterface
There was a problem hiding this comment.
IUntil now i have seen the following patterns for handlers / extractors / converter … thingys
- A chain of objects that get asked wether they can handle the type
- A configuration that specifies the extractor for each type
The approach here would be a third pattern but it has the clear benefit allowing a really type safe implementation. Only downside is that there is no delegation as everything is implemented by the object itself. Actually this might be the biggest advantage aswell.
I think in total it makes sense. If there is prior art of this pattern in the codebase i would agree even more
github-actions
bot
added
9.1
and removed
feature/new-link-editor-link-value-object
labels
3 participants
Followup to #5682
Separated because discussions might block main thread.
Upgrade instructions
Review instructions
Checklist
FEATURE|TASK|BUGFIX!!!and have upgrade-instructions