Skip to main content
added 404 characters in body
Source Link
Tom J Nowell
  • 60.6k
  • 7
  • 77
  • 147

If you're going to create classes and objects, do it because you need to. If you don't plan to create more than one of an object, and your class has no internal state, it's usually a sign you don't need a class/objects for the task. Namespaces will serve the same purpose just as well. A lot of developers think that as they get more experienced everything becomes objects and OOP, and use classes for everything regardless of what it is or wether it benefits tem, but that's not true. OOP is a tool that can be very useful, but it's not a universal hammer that does everything, use it when it's needed but for most things in WordPress like simple shortcodes or action hooks it's overkill.

If you're going to create classes and objects, do it because you need to. If you don't plan to create more than one of an object, and your class has no internal state, it's usually a sign you don't need a class/objects for the task. Namespaces will serve the same purpose just as well.

If you're going to create classes and objects, do it because you need to. If you don't plan to create more than one of an object, and your class has no internal state, it's usually a sign you don't need a class/objects for the task. Namespaces will serve the same purpose just as well. A lot of developers think that as they get more experienced everything becomes objects and OOP, and use classes for everything regardless of what it is or wether it benefits tem, but that's not true. OOP is a tool that can be very useful, but it's not a universal hammer that does everything, use it when it's needed but for most things in WordPress like simple shortcodes or action hooks it's overkill.

Source Link
Tom J Nowell
  • 60.6k
  • 7
  • 77
  • 147

The problem is that your JS needs to know the post type for each shortcode, but rather than looking at the shortcodes output, it looks at the main global kne_ajax_object, and since there is only one kne_ajax_object, there can only be one post type.

Additionally, your javascript code only runs one time, and only makes one AJAX call, which means there can only be one post type and one shortcode.

Instead, store the AJAX URL/security stuff in kne_ajax_object but put the rest of the information on the HTML the shortcode generates.

E.g. instead of this:

<div class="kne-posts-container-<?php echo $kne_unique ?>">

Why not this?

<div class="kne-posts-container" data-post-type="mycpt">

Then do the equivalent of this in JS:

foreach element that matches `.kne-posts-container`
    post type = element.data('post-type')
    do the ajax call
        on success
            element.append( result )

Or even better, use the REST APIs that WordPress already provides. WP already has AJAX friendly data only URLs that return all this information as JSON, with pretty URLs like example.com/wp-json/wp/v2/posts.

Additional Notes

echo the_title('<h3>', '</h3>', true);

the_title already echo's so this is the same as echo echo get_title(, you don't need to use echo on most functions that begin with the_

Using -1 for posts_per_page is bad, I know you want to list all posts, but can your server load an infinite number of posts? If it can handle 500 or less posts, why not set it to 500? Why chance that your server will fallover trying to send thousands of posts? Set posts_per_page to a high number you never expect to reach, but you know your server can actually handle.

There's no point using offset, this isn't going to apply some old MySQL pagination speed trick. Just used the standard parameter 'paged' => $page, it might even be faster.

Finally, escaping. This is bad code:

echo '<a href="'. $url . '">';

Are you sure $url is a URL? Do you trust that? What if your site was hacked and it actually contains "><script>nasty stuff goes here</script>?

Instead escape:

echo '<a href="'. esc_url( $url ) . '">';

Now it is always a URL, it is guaranteed, we no longer need to trust $url. If it contains something nasty, it'll be neutered, cut down to a URL shaped size, and the worst case scenario is now a broken link.

Finally, this:

$slider = new KNE_DATA_SLIDER();
add_shortcode('kne_data_slider', array($slider, 'shortcode'));

This is not object oriented programming, it's just functions being put inside a class. If you removed the class and turned them into pure functions it would work exactly the same. It's just additional boilerplate.

If you're going to create classes and objects, do it because you need to. If you don't plan to create more than one of an object, and your class has no internal state, it's usually a sign you don't need a class/objects for the task. Namespaces will serve the same purpose just as well.