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

Turn NodeType into a less magic self-translating, lazy-loading beast -> readonly immutable #4523

Closed
mhsdesign opened this issue Sep 16, 2023 · 3 comments
Labels

Comments

@mhsdesign
Copy link
Member

mhsdesign commented Sep 16, 2023

those were Sebastian’s todos in the code

TODO: REFACTOR TO immutable readonly; and value objects

TODO: I'd love to make NodeType final; but this breaks quite some unit and functional tests.

also isOfType should ideally be cached by creating a map of all nodetypes at once? … At least the method should accept the DTO name ;) #4556

it would be cool to still have some lazy evaluations, which are possible via this trick https://peakd.com/hive-168588/@crell/php-tricks-lazy-public-readonly-properties -> no probably not a good idea as discussed on slack

Unset hack in action:
final readonly class NodeType
{
    public NodeTypeName $name;
    public bool $isAbstract;
    public bool $isFinal;

    public bool $isAggregate;
    public string $label;

    public array $superTypes;
    public array $properties;
    public array $childNodes;
    public array $options;
    public NodeLabelGeneratorInterface $nodeLabelGenerator;

    // internal
    private array $fullConfiguration;
    private NodeLabelGeneratorFactoryInterface $nodeLabelGeneratorFactory;

    public function __construct(
        NodeTypeName $name,
        array $superTypes,
        array $configuration,
        NodeLabelGeneratorFactoryInterface $nodeLabelGeneratorFactory
    ) {
        /** eager properties */
        $this->name = $name;
        $this->superTypes = $superTypes;
        $this->abstract = ($configuration['abstract'] ?? null) === true;
        $this->final = ($configuration['final'] ?? null) === true;

        /** internal eager properties */
        $this->fullConfiguration = $configuration;
        $this->nodeLabelGeneratorFactory = $nodeLabelGeneratorFactory;

        /** lazy properties {@see __get()} */
        unset($this->isAggregate);
        unset($this->properties);
        unset($this->childNodes);
        unset($this->options);
        unset($this->nodeLabelGenerator);
    }

    /**
     * We unset the readonly properties in the constructor, so that this magic getter is invoked, which initializes the properties.
     */
    public function __get(string $key)
    {
        $this->initialize();
        return $this->$key = match($key) {
            'isAggregate' => ($this->fullConfiguration['aggregate'] ?? null) === true,
            'properties' => $this->fullConfiguration['properties'] ?? [],
            'childNodes' => $this->fullConfiguration['childNodes'] ?? [],
            'options' => $this->fullConfiguration['options'] ?? [],
            'nodeLabelGenerator' => $this->nodeLabelGeneratorFactory->create($this)
        };
    }

related: #4515

related: #4228

@mhsdesign mhsdesign added the 9.0 label Sep 16, 2023
@mhsdesign mhsdesign changed the title Make nodetype immutable readonly model Turn NodeType into a less magic self-translating, lazy-loading beast -> readonly immutable Sep 29, 2023
@mhsdesign
Copy link
Member Author

mhsdesign commented Oct 10, 2023

We should also get rid of the implicit object manager dependency when using new for the post-processors.

... otherwise frameworks, applications without magic object manager, will have not chance in instantiating them with dependencies.

@bwaidelich
Copy link
Member

@mhsdesign I collected some ideas with #4228 (comment)
Are you OK to coordinate these topics with #4228 and close this one?

@mhsdesign
Copy link
Member Author

Yes sure ;) Also the part to make the NodeType readonly was implemented via #4677 and the proposed lazy hack turned out to be a bad idea. So this discussion is done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

No branches or pull requests

2 participants