ns-rx / poetwp

0 stars 0 forks source link

Include hooks #8

Open ns-rx opened 3 months ago

ns-rx commented 3 months ago

The plugin should be extensible and include hooks where it makes sense, should it need to be used on an existing site with poems that need exposing.

GaryJones commented 3 months ago

Separating data from what we do with it

An example here would be during the CPT registration. Maybe someone wants to amend the labels (and not just translate the string), or amend some other CPT attributes. Currently, it's like:

function poetwp_register_post_type() {
    register_post_type(
        'poem',
        array(
            'labels'       => array(
                'name'          => __( 'Poems' ),
                'singular_name' => __( 'Poem' ),
            ),
            'public'       => true,
            'has_archive'  => true,
            'supports'     => array( 'title', 'editor', 'thumbnail', 'author', 'excerpt' ),
            'menu_icon'    => 'dashicons-format-aside',
            'rewrite'      => array(
                'slug' => 'poems',
            ),
            'show_in_rest' => true,
        )
    );
}

Here, we can see that the key bits of data is mixed in directly to the register_post_type() function call (which makes it multi-line and therefore harder to read). The whole outer callback function also gets nested up to 4 levels deep in some cases (the more indentation levels you have, the more complex / code smell it should be taken as). Consider refactoring (same behaviour) to pull the data out into a standalone array first:

function poetwp_register_post_type() {

    $post_type_args = array(
        'labels'       => array(
            'name'          => __( 'Poems' ),
            'singular_name' => __( 'Poem' ),
        ),
        'public'       => true,
        'has_archive'  => true,
        'supports'     => array( 'title', 'editor', 'thumbnail', 'author', 'excerpt' ),
        'menu_icon'    => 'dashicons-format-aside',
        'rewrite'      => array(
            'slug' => 'poems',
        ),
        'show_in_rest' => true,
    );

    register_post_type( 'poem', $post_type_args );
}

Now we've separated the data from what we're going to do with it, then we can manipulate the data first, such as by allowing it to be filtered:

function poetwp_register_post_type() {

    $post_type_args = array(
        'labels'       => array(
            'name'          => __( 'Poems' ),
            'singular_name' => __( 'Poem' ),
        ),
        'public'       => true,
        'has_archive'  => true,
        'supports'     => array( 'title', 'editor', 'thumbnail', 'author', 'excerpt' ),
        'menu_icon'    => 'dashicons-format-aside',
        'rewrite'      => array(
            'slug' => 'poems',
        ),
        'show_in_rest' => true,
    );

    /**
     * Filter the args for the poem custom post type.
     * 
     * ...(there's a defined format for noting the arguments that I'm skipping here)
     */
    $post_type_args = (array) apply_filters( 'poemwp_post_type_args', $post_type_args );

    register_post_type( 'poem', $post_type_args );
}

Separating data definition to its own function

Now, we can see that 90% of this "register" function is about setting up the args, rather than the registering itself (final line). We can consider extracting the args definitions and filtering to it's own function:

function poetwp_post_type_args() {

    $post_type_args = array(
        'labels'       => array(
            'name'          => __( 'Poems', 'poetwp' ),
            'singular_name' => __( 'Poem', 'poetwp' ),
        ),
        'public'       => true,
        'has_archive'  => true,
        'supports'     => array( 'title', 'editor', 'thumbnail', 'author', 'excerpt' ),
        'menu_icon'    => 'dashicons-format-aside',
        'rewrite'      => array(
            'slug' => 'poems',
        ),
        'show_in_rest' => true,
    );

    /**
     * Filter the args for the poem custom post type.
     * 
     * ...(there's a defined format for noting the arguments that I'm skipping here)
     */
    return (array) apply_filters( 'poemwp_post_type_args', $post_type_args );
}

function poetwp_register_post_type() {
    register_post_type( 'poem', poetwp_post_type_args() );
}

Now the args data can be reused as needed, and unit tests can be set up against it, without the test having the side effect of registering the post type in our test system.

Constants vs functions

Since the 'poem' post type is such an important value (and re-used in a number of places I imagine), then setting it up as a constant, or better, returning it from it's own function, would be beneficial. Why is a function better? Because then we have more forward-flexibility for future changes.

Let's take it as a constant first:

define( 'POEMWP_CPT_KEY', 'poem' );

function poetwp_register_post_type() {
    register_post_type( POEMWP_CPT_KEY, poetwp_post_type_args() );
}

That's great, and the constant can be used elsewhere too, but, what if we need to add a filter to the key, or want to make it conditional on something? We can't amend a constant, as it's a constant! So let's use a function everywhere it's needed instead:

function poetwp_post_type_key() {
    return 'poem';
}

function poetwp_register_post_type() {
    register_post_type( poetwp_post_type_key(), poetwp_post_type_args() );
}

Now we can change the first function, without having to touch everywhere that references it (Single Responsibility Principle):

function poetwp_post_type_key() {
    /** ... */
    return apply_filters( 'poemwp_post_type_key', 'poem' );
}

function poetwp_register_post_type() { 
    register_post_type( poetwp_post_type_key(), poetwp_post_type_args() ); // No change here.
}

At the end of that round of refactoring, we've gone from one function that is closed to modification to three functions (poetwp_post_type_key(), poetwp_post_type_args(), and poetwp_register_post_type()), two of which are filterable right before they return a value, and the last of which is a callback hooked into to init and a one-line call to a WP function.

Wrapping it up as a class

To take it into class structure (not really OOP, since it's a single thing), you could imagine a class that handles all of this highly related behaviour (I've left out the formal docs for convenience, but added extra explanatory notes):

<?php

namespace PoemWP;

// Use of `final` here means the class can't be extended - we want any customisations to use the provided hooks.
final class Post_Type {
    // The default can still be a class constant so that it stands out / isn't buried in a method, but we make it private
    // so that it can be called directly; we want the getter method to be used.
    private const KEY = 'poem';

    // We add the hook into a run() method, rather than a constructor, so we can instantiate the class without
    // having unintended side effects.
    public function run(): void {
        // This call uses the first-class callable feature of PHP 8.1 instead of the usual array( $this, '...' ) format.
        // See https://www.php.net/manual/en/functions.first_class_callable_syntax.php
        add_action( 'init', $this->register_post_type() );
    }

    public function register_post_type(): void {
        register_post_type( $this->post_type_key(), $this->poetwp_post_type_args(...) );
    }

    public function post_type_key(): string {
        /** ... */
        return apply_filters( 'poemwp_post_type_key', self::KEY );
    }

    public function post_type_args(): array {
        $post_type_args = array(
            'labels'       => array(
                'name'          => __( 'Poems', 'poetwp' ),
                'singular_name' => __( 'Poem', 'poetwp' ),
            ),
            'public'       => true,
            'has_archive'  => true,
            'supports'     => array( 'title', 'editor', 'thumbnail', 'author', 'excerpt' ),
            'menu_icon'    => 'dashicons-format-aside',
            'rewrite'      => array(
                'slug' => 'poems',
            ),
            'show_in_rest' => true,
        );

        /**
         * Filter the args for the poem custom post type.
         *
         * ...(there's a defined format for noting the arguments that I'm skipping here)
         */
        return (array) apply_filters( 'poemwp_post_type_args', $post_type_args );
    }
}

Then, in our plugin root file, we can add:

$poemwp_post_type = new PoemWP\Post_Type();
$poemwp_post_type->run();

Thinking in more OOP

In OOP, you want to be thinking of things and actions (or services). So, our entity thing here is the post type. It will have a key, and some attributes. That's it.

Separately, we want to be able register that post type with WP - that's the action/service. So we should split up our one class into two (and since this is more OOP, we could start using PSR-4 file and class naming to group the related classes - it's against the WordPressCS, but it's more scalable when interfaces, traits, enums and other structures are used):

// src/PostType/Post_Type.php
namespace PoemWP\PostType;

final class Post_Type {
    public const KEY = 'poem';

    public static function key(): string {
        /** ... */
        return apply_filters( 'poemwp_post_type_key', self::KEY );
    }

    public static function args(): array {
        $args = array(
            'labels'       => array(
                'name'          => __( 'Poems', 'poetwp' ),
                'singular_name' => __( 'Poem', 'poetwp' ),
            ),
            'public'       => true,
            'has_archive'  => true,
            'supports'     => array( 'title', 'editor', 'thumbnail', 'author', 'excerpt' ),
            'menu_icon'    => 'dashicons-format-aside',
            'rewrite'      => array(
                'slug' => 'poems',
            ),
            'show_in_rest' => true,
        );

        /**
         * Filter the args for the poem custom post type.
         *
         * ...(there's a defined format for noting the arguments that I'm skipping here)
         */
        return (array) apply_filters( 'poemwp_post_type_args', $args );
    }
}
// src/PostType/Register.php
namespace PoemWP\PostType;

final class Register {

    public function run(): void {
        add_action( 'init', $this->register_post_type(...) );
    }

    public function register_post_type(): void {
        register_post_type( Post_Type::key(), Post_Type::args() );
    }
}

Then the root file has:

$poemwp_post_type_register = new Register();
$poemwp_post_type_register->run();

You could imagine that there would be an Unregister service class too, or that if a plugin had multiple post types to register, that the Register class would accept multiple post type objects (as a collection) and the run() method would loop through and register all of them...

Conclusion

Are these multiple functions/methods, multiple classes, and multiple files more complex than a single function and callback? Most definitely, and for a plugin this small, it's probably over-engineered. But, we've allowed for: