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

[DX] Remove direct use of new Node() and other direct instantiations of entity classes from the codebase #6075

Open
argiepiano opened this issue Apr 21, 2023 · 4 comments

Comments

@argiepiano
Copy link

argiepiano commented Apr 21, 2023

When Backdrop partially incorporated D7 Entity API into core, and switched to using named classes for its core entities (that is, Node and User etc instead of stdClass), this allowed developers to extend those entity class (and their entity controllers) to incorporate additional functionalities and properties. This can be done very simply and efficiently, by (1) implementing a hook_entity_info_alter() to change the entity class and the entity controller, and (2) extending the entity class and/or the entity controller class with the new functionality or properties.

Unfortunately, at the same time, a few places in Backdrop codebase instantiate those core entity classes by making direct calls, as in $node = new Node();. This effectively curtails the new possibilities created by the use of named classes as described above. So, if you are a developer who wants to transform/expand nodes in new ways, you are limited by those few direct instantiations (so, in those places, the resulting entity of calling $node = new Node(); will use the core class, instead of the class you created to extend nodes).

The solution here is to replace those few new Node() and similar calls, with the intermediary API function entity_create('node');. This call will take care of instantiating the entity class as defined in the entity_info_alter hook.

There are very few direct instantiations of core entity classes in the codebase:

  • 7 for Node()
  • 4 for User()
  • 22 for File()
  • 2 for Comment()
  • 6 for TaxonomyTerm()

Many of those are in tests, which we can simply leave alone, as we want to use the core classes in tests.

@argiepiano argiepiano changed the title Remove direct use of new Node() and other entity classes from the codebase Remove direct use of new Node() and other direct instantiation of entity classes from the codebase Apr 21, 2023
@argiepiano argiepiano changed the title Remove direct use of new Node() and other direct instantiation of entity classes from the codebase Remove direct use of new Node() and other direct instantiations of entity classes from the codebase Apr 21, 2023
@klonos klonos changed the title Remove direct use of new Node() and other direct instantiations of entity classes from the codebase [DX] Remove direct use of new Node() and other direct instantiations of entity classes from the codebase Apr 22, 2023
@argiepiano
Copy link
Author

argiepiano commented Apr 24, 2023

Further, Drupal discourages instantiating classes directly. Instead their coding standards ask for the use of factory functions - such as entity_create() of entity_get_controller('node')->create().

@kiamlaluno
Copy link
Member

@kiamlaluno
Copy link
Member

kiamlaluno commented Apr 24, 2023

(Please disregard this comment. I apologize if it came out inappropriate or offensive toward a specific community.)

@argiepiano
Copy link
Author

argiepiano commented Apr 24, 2023

@kiamlaluno the example you provide actually does not instantiate a Node object directly, so it doesn't break that rule. That code calls a static factory method, which is one of the ways to avoid using direct instantiation.

Direct instantiation is when you use new Something() to instantiate an object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants