6

Over at How to get term name from a tid? the accepted answer for "how do I get the name for a Term" is this:

use Drupal\taxonomy\Entity\Term;
...
$term = Term::load($tid);
...

That's really nice, easy to read code (and very easy to remember). It's also, as it turns out, applicable to all entity types in Drupal (just substitute the Term class for the type of entity you want).

Why then, do the overwhelming majority of the code examples and tutorials published around the place seem to advocate this monstrosity? ...

$term = \Drupal::entityTypeManager()->getStorage('taxonomy_term')->load($tid);

That approach bakes in one of my [many] grievances with PHP in general - it's full of magic strings! How the hell am I supposed to know at "compile time" whether 'taxonomy_term' is the right argument to getStorage()?? There's not a PHP IDE in the land that'll catch me sticking 'taxonomy' in there instead.

Looking at the contents of that ::load method (which is defined in \Drupal\Core\Entity\Entity and shared by all Drupal entities), it's got all the dependency-injection cleverness of that second-approach - plus it fills in the right argument to getStorage() for you automatically!

So why does more of the Drupal community not use it? Is there something I'm missing? Or is it just to teach us "first principles" so that we can write shared code that works with any entity type? (you'd need the long-hand way in a hook that was intended to work across all entity types).

7

There is actually an issue about adding this info to the docs.

In short, the long winded version is unit test friendly.

A comment by one of the Core's contributors fully detailed explains why. Just to highlight the main point:

If we call Node::load() in OO code we are breaking this principle (dependency inversion), because we are hardcoding the specific implementation (\Drupal\node\Entity\Node::load() to be exact). This code path is hard coupled to \Drupal::entityManager() which is hard coupled to \Drupal::$container. So now our code becomes impossible to run outside of Drupal.

Furthermore, since code that uses the dependency inversion principle is fully standalone it is very easy to unit test it by mocking the dependencies according to the requested interfaces. If we introduce Node::load() then it becomes really hard to "properly" unit test the class, since it will start polluting the test environment with external code, and the dependency injection container is not available in unit tests.

Now if you wonder then why does Entity::load exists

There were many loud voices at the time that D8 was going to be a DX disaster, that the OO approach was too difficult to understand and write, that it would never get adopted by the wider community. There was a huge push going on to make D8 more DX friendly, mainly driven by the Backdrop fork that was announced a few months earlier.

So we created these static methods that wrap the loading of the entity manager from the container, and can be called directly from the procedural side. It was great, since it offers a drop-in replacement that works and looks very much like the original code. This made porting of procedural D7 code vastly easier:

// Original D7 procedural code. 
$node = node_load($nid);

// New D8 procedural code. 
$node = Node::load($nid);
  • Thanks "No Sssweat", that explains it all very nicely. Since the only "fixed" thing is the entityTypeManager, I wonder if the ::load($id) method couldn't be re-defined as ::load($id, $container = null) allowing a container as an optional argument, and defaulting to the \Drupal:entityTypeManager() inside the method implementation. I'm all for OO code (I've written a lot of it in a lot of frameworks), but geez it can be crazy-long winded sometimes (and in this case there's a horrible magic string in the long-winded version, which is completely avoided in the static method) – Daniel Scott Oct 4 '18 at 14:37
2

I think this is about the target audience of the code example. If it is for a hook, then I agree, use the first more easy to read approach.

If it is for a service, then the correct approach would be to inject the entity type manager and use it instead of the static \Drupal wrapper.

$term = $this->entityTypeManager->getStorage('taxonomy_term')->load($tid);

Now the question is what if you want to provide a more universal code example, then

$term = \Drupal::entityTypeManager()->getStorage('taxonomy_term')->load($tid);

is probably the best approach, because it works in procedural code if you copy and paste it and can be easily adapted for a service where you want to inject the dependencies.

  • Thanks 4k4: The note about writing a service (or, indeed a controller) is a good one. – Daniel Scott Oct 4 '18 at 14:41

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.