Closed DiegoPino closed 3 years ago
@DiegoPino a whole lot of Drupal passes pointers to objects between functions because it requires less overhead than passing actual data. I'm often amazed at the amount of stuff that is provided as context in functions until I remember this - it's just pointers. The object (node) being generated is in memory already, so wouldn't it be most efficient to just pass the reference to it into this alter hook?
As for the alterability of the referenced object, it seems to me that this is a matter of developer beware. May be a horrible idea. But not our concern?
@patdunlavey I'm already refactoring a lot so at least not in this pass. Passing the whole node around (which we do on the event itself really) would mean moving all the preserving file logic/danger into the Service, and I'm not ready to allow external developers to jeopardize the most important aspect of a repositories: metadata consistency and file saving, which is what is in play here. Feel free to experiment refactoring my pull request if you want, happy to review the code. But the fact that we have here a very specific concern (file saving and classification) makes me thing "opening" the door to other hidden alters may be bad. I do agree on the memory aspect of things. And that is what we do "most" of the time.
What?
Refactor. Refactor. Maybe some altering entity needs to use the "title" of an ADO to define its final saving structure. So we may pass the data to the hook too, in case that is something people desires. (Slack says someone desires this)
I won't pass the actual
Field
object (the strawberry field) because it could end being altered by the altering hook and that my friends is a bad idea? (or is that desired?) Also won't clone the Object because that is a lot of memory wasted (then again passing an array is also a lot of memory?)