Sometimes we need a function to run on every page load, so as i understand there are to options:
- hook it with(say
init) - Call directly within functions.php
when it would be good to use one of them.
because i have this code which i need to run always to check condition and do stuff, so i added to functions.php like this
$args = [
'post_type'=> 'product',
'numberposts'=> -1,
'meta_key'=> 'hfx_datepicker',
'fields' =>'ids',
];
$products = get_posts($args);
foreach($products as $productID){
$event = 'workshop_event_schedual_'.$productID;
if (! has_action ( $event )) {
add_action($event, function() use($productID){
if(!get_option('student_expiry_set_'.$productID)){
set_student_workshop_expiry($productID);
add_option('student_expiry_set_'.$productID, true);
}
product_purchaser_notify($productID);
});
}
}
So i had do think why don't use init but no change will happen, and i started to ask myself which is best practice. When to be recommended to choose one over another?
Sometimes we need a function to run on every page load, so as i understand there are to options:
- hook it with(say
init) - Call directly within functions.php
when it would be good to use one of them.
because i have this code which i need to run always to check condition and do stuff, so i added to functions.php like this
$args = [
'post_type'=> 'product',
'numberposts'=> -1,
'meta_key'=> 'hfx_datepicker',
'fields' =>'ids',
];
$products = get_posts($args);
foreach($products as $productID){
$event = 'workshop_event_schedual_'.$productID;
if (! has_action ( $event )) {
add_action($event, function() use($productID){
if(!get_option('student_expiry_set_'.$productID)){
set_student_workshop_expiry($productID);
add_option('student_expiry_set_'.$productID, true);
}
product_purchaser_notify($productID);
});
}
}
So i had do think why don't use init but no change will happen, and i started to ask myself which is best practice. When to be recommended to choose one over another?
1 Answer
Reset to default 2If we take your code at face value, there are some consequences:
- it runs on every request, be that a page, an AJAX call, a REST API request, even XMLRPC
- It's a super expensive post query that involves post meta, with no upper limit on the number of results
- You are filtering a post query by a posts meta values, but that's not what post meta is for. That's what the taxonomy tables are for. If you ever need to search for a post by it's X or Y, then X/Y need to be custom taxonomies, or you'll get awful performance.
scheduleis misspelt- Options are being used to store data that should be stored as either user meta or post meta ( or in this case, as a taxonomy term )
So you're already in a bad place as far as performance is concerned, and to top it off, because it's in functions.php and not in any kind of functions, it can't be overrode or unhooked by a child theme or plugin.
Instead by only firing it on a hook we separate the "what" from the "when", and can conditionally hook it in.
For example, you could hook into admin_init if it only runs on the backend.
By hooking it in, that also means it can be unhooked if needed
But looking at what you're actually doing here, this will not scale. As the number of product posts increases, the site will get slower and slower ( regardless of how many results are found, it's the searching that's expensive )
What's more, this shouldn't be happening on every page load, it should be happening in a cron job and in batches.
Additionally, by relying on the options table to set expiry times, your options table will balloon in size. It'll quickly become incompatible or slow with a lot of object cache drop ins due to the way auto-loaded options are cached and loaded on every page request.
All in all, I would expect this to work fine with 4 or 5 products, but quickly get out of hand once it goes past 100 posts. There are significant performance gains to be had by changing how this works