While performing a code review for some work I had done, I was asked the following question by Darren regarding a new method I had added to EEM_Registration called event_reg_count_for_statuses() :
I wonder if it’d be better to have the first argument be an $Event ID instead of a full event object?
The method in question type hinted for an Event object:
1 |
public function event_reg_count_for_statuses(EE_Event $event, $statuses = array() ) { |
then used that event object to populate part of a query’s where conditions like :
1 |
WHERE wp_esp_event.EVT_ID = ### |
in which we would substitute ### for the Event ID using
1 |
$event->ID() |
Now normally it’s advisable to try and type hint for objects as much as possible, but keeping in mind that this method was basically a query helper on the EEM_Registration model used for retrieving information from the database, Darren’s suggestion was actually a better approach.
But why?
Function Parameters : Primitives or Objects ?
As stated above, it’s normally advisable to try and type hint for objects as much as possible because there are many benefits to be gained from passing an object instead of any other alternative. The following are some musings on why this is usually the case, written using the method discussed above as a point of reference.
Guaranteed Identity
If a function accepts a value that we can not type hint for, then we really don’t know what we are getting. Even if an int is received (as opposed to some other value)… how do we know that it is a valid Event ID ? It could be any random number, and no amount of validation logic ensuring that we receive an int can change that it could still be a random number. But by simply type hinting for an EE_Event, we have guaranteed that the value we pass along to the query is not only an int, but a valid Event ID. No further validation logic beyond the type hint is necessary but the method gains a lot of stability.
Efficiency and Memory Usage
In PHP all objects are passed by reference, whereas passing an int would require a new local variable to be created to hold a copy of an already existing variable, so more memory is required as there will be more entries on PHP’s memory stack. This isn’t a big deal if there are only two methods involved, and the one receiving the Event ID to perform the query isn’t passing that ID along anywhere else (other than the model). But if the chain of methods involved was bigger, and the Event ID was getting passed around a lot, then the memory usage for that one bit of data can grow quickly. Change your variable to an array of data and the inefficiency goes way way up. When I first started with EE working on EE3, the methods involved in the registration process would all pass IDs amongst themselves. This was horrifically inefficient because you would have a method receive an Event ID, then query for the Event object so that it could get other data for the Event, then pass the ID along to the next method, that would also query for the Event object so that it too could get other data for the Event. This sometimes happened 4-5-6 times in a row, meaning 4-5-6 queries for an Event object that was already in the system. And many of these methods would also save their changes to the Event object before passing its ID along, so a series of function calls could result in dozens of unnecessary additional queries (we can thank the previous lead dev Abel for that bit of amazing code /sarcasm). This is the worst case scenario and the main thing that I want to avoid, so if you are writing a method that requires an object, you should type hint for that object instead of simply requesting an ID. Then any other methods that require the use of your method can handle obtaining the object based on what data they have, assuming they don’t already have it.
Fail Early at the Source
So when a function type hints for an object, then it can only receive a valid instance of that object. Where the object was originally instantiated is inconsequential because PHP is only passing pointers around instead of the actual object, and if a valid object could not be created, the error would likely be discovered immediately. But when you pass primitive data types around, it can sometimes be difficult to determine where the data originated from.
If a function requires a valid valid Event ID but some invalid value is received instead, then we are farther away from the original source of the invalid data. By forcing the code that first obtained the int to retrieve an Event, any errors can be thrown at the source of the problem. I know I have experienced difficulties in finding the source of a problem because some variable was passed around through some filters and/or actions, and by the time the variable was discovered to be invalid, the source no longer appeared in the stack trace. This kind of follows the fail early philosophy. In an ideal application, the request data would be validated immediately and any invalid data would produce errors immediately. By passing an integer through the system, we can not do this. Of course it’s not always ideal to convert an int to an object just so that you can retrieve that same int again from the object (especially if a query was required to build the object).
Domain Driven Design
In articles discussing Domain Driven Design, you will often see images of a system represented by a series of rings or hexagons that surround each other. The innermost ring represents the domain where all of the business logic resides. The domain should be completely ignorant of the request as that interaction should be handled by one of the outer rings. Imagine the stability of a domain that ONLY type hints for objects as opposed to one that allows primitive data types to be passed around. The second would require significantly more validation and processing to ensure that the methods were receiving the data that they require. Whereas the domain that exclusively type hints for objects would be much much more stable, since the range of data its methods could accept would be greatly reduced. In this kind of system, your outer ring that interacts with the incoming request would be responsible for validating all of the incoming data before passing it to the domain. So your controller type classes that receive the user input or request data, would also need to have access to a model or repository that they could pull the appropriate object from before passing things along to the inner domain. In this situation, since event_reg_count_for_statuses()
is a query helper method on the model, it’s appropriate for it to receive an ID instead of an object. So in this case, we decided not to OOP.