Funky stuff between Plugin and CPT classes

[csrui]

The way CPT classes are handled here is kinda funky. The CPT classes seem to be tightly coupled with the plugin. It's shit for testing and general bad practice. Anyone agrees?

[Narayon]

I'm not sure what you mean. Can you expand your idea?

[csrui]

Ok, lets check this example. I'm creating a new post type called event. On the register method we have a bunch of arguments that are defined by this class. Why in world would want to define the slug outside of this class? We should either define everything inside the class or everything outside. It seems a bit nonsense to do it both ways.

Because of this, the two classes Event & Plugin that become coupled which is something we must run away for the sake of simplicity and testing. Also why would the Plugin class need to know the slugs in order to register the CPT's. It only needs to know that there is a method called register and call it.

Example 1 (how it is now).

<?php

/**
 * Class for register a banner post type.
 *
 * @link       https://bitbucket.org/gulbenkian/fcg_events_calendar
 * @since      1.0.0
 *
 * @package    Gulbenkian/PostType
 */

namespace Gulbenkian\WP\Plugin\FcgEventsCalendar\PostType;

use Gulbenkian\WP\Plugin\FcgEventsCalendar\PostType;
use Gulbenkian\WP\Plugin\FcgEventsCalendar\Plugin;

/**
 * Register a banner post type.
 *
 * @since      1.0.0
 * @package    Gulbenkian
 * @author     log.OSCON, Lda. <engenharia@log.pt>
 */
class Event extends PostType {

    /**
     * Register the custom post type.
     *
     * @since    1.0.0
     */
    public function register() {

        $rewrite = array(
            'slug' => Plugin::CPT_SLUG_NAME,
        );

        $args = array(
            'label'                 => \__( 'Event', 'fcg-events-calendar' ),
            'description'           => \__( 'Event page', 'fcg-events-calendar' ),
            'labels'                => array(),
            'hierarchical'          => true,
            'public'                => true,
            'show_ui'               => true,
            'show_in_menu'          => true,
            'menu_position'         => 5,
            'rewrite'               => $rewrite,
        );

        \register_post_type( Plugin::CPT_SLUG_NAME, $args );
    }

    /**
     * Register hooks.
     *
     * @since    1.0.0
     */
    public function register_hooks() { }
}

Example 2 (how it would be better).

<?php

/**
 * Class for register a banner post type.
 *
 * @link       https://bitbucket.org/gulbenkian/fcg_events_calendar
 * @since      1.0.0
 *
 * @package    Gulbenkian/PostType
 */

namespace Gulbenkian\WP\Plugin\FcgEventsCalendar\PostType;

use Gulbenkian\WP\Plugin\FcgEventsCalendar\PostType;

/**
 * Register a banner post type.
 *
 * @since      1.0.0
 * @package    Gulbenkian
 * @author     log.OSCON, Lda. <engenharia@log.pt>
 */
class Event extends PostType {

    protected $slug = 'bananas';

    /**
     * Register the custom post type.
     *
     * @since    1.0.0
     */
    public function register() {

        $rewrite = array(
            'slug' => $this->get_slug(),
        );

        $args = array(
            'label'                 => \__( 'Event', 'fcg-events-calendar' ),
            'description'           => \__( 'Event page', 'fcg-events-calendar' ),
            'labels'                => array(),
            'hierarchical'          => true,
            'public'                => true,
            'show_ui'               => true,
            'show_in_menu'          => true,
            'menu_position'         => 5,
            'rewrite'               => $rewrite,
        );

        \register_post_type( $this->get_slug(), $args );
    }

    /**
     * Register hooks.
     *
     * @since    1.0.0
     */
    public function register_hooks() { }
}

[Narayon]

That's a valid point and I agree with the way of thinking, but there's a problem that is solved by the current approach:
Imagine that you need the CPT slug in other parts of the plugin(taxonomies, fields, etc.) or even in other plugins, you would have to instantiate the CPT class and make the slug getter public available, just to know the slug. That's an even worse example of coupling and a red flag for other reasons.

Moreover, the constant is just a reference for reusability, is not really a property of the class, but even if it was, you can look at the Plugin class kinda like a factory class, the main abstraction for all the processes that need to be done when creating the plugin.

In your example, you can remove the reference to the Plugin, by invoking the property slug of the parent class, PostType, but ideally, I would create a private getter. By doing that, there's no coupling anymore, because the slug is injected on creation and neither the abstract class or the concrete class have any references to the Plugin class.

Thank you for opening the discussion about the design of this generator. I think we need to improve it, in some aspects, and talking about it is always a good starting point.

[csrui]

The purpose is to see how we can improve the code. I hope all pitch in since this a core part of the code we daily use. :fofinho: :fofinho: :fofinho: :fofinho:

I'll comment in parts for easier reading

Imagine that you need the CPT slug in other parts of the plugin(taxonomies, fields, etc.) or even in other plugins, you would have to instantiate the CPT class and make the slug getter public available, just to know the slug. That's an even worse example of coupling and a red flag for other reasons.

Indeed. The example I provided should have been with a static class & or property on the Event class.

you can look at the Plugin class kinda like a factory class

No, sorry but I cant :) (http://www.phptherightway.com/pages/Design-Patterns.html).

In your example, you can remove the reference to the Plugin, by invoking the property slug of the parent class, PostType, but ideally, I would create a private getter. By doing that, there's no coupling anymore, because the slug is injected on creation and neither the abstract class or the concrete class have any references to the Plugin class.

Yes, indeed. I'll start spanking @lzcalderaro right now for his PR on which I based my original example. If PostType had a protected getter, it would be a nice addition yes. That would take care of one part of the "problem" but still...

Its more logical to call \Namespace\PostType\Event::slug that \Namespace\Plugin::event_slug

[Narayon]

Its more logical to call \Namespace\PostType\Event::slug that \Namespace\Plugin::event_slug

That's a very valid point. I guess the constants are all in the Plugin class for practicality and that's never a good enough reason.

[RicCastelhano]

Valid points in both statements. Should we be waiting soon for a pull-request with the improved code ? 😈

[csrui]

Yeah... just discussing things first. Still would have to spend time understanding yeoman to do so.

I'll follow up with another question. Why pass Plugin as a parameter of the class PostType? Is it just for the slug or some other thing?

[csrui]

While no feedback was offered, I just wanted to let you know that a new / reworked "plugin framework" is on the works to be presented maybe early next year. I'd ❤️ to hear the pain points of each dev with the current one but have no ideia how to collect them. Any suggestions?

[Narayon]

While no feedback was offered

We've been talking here for a while. Not sure what you mean.

I would love to hear what you have in mind for that new plugin framework and in what manners will improve the current one. Maybe I can help you build those improvements. I have some thoughts and references for improvements, like adding a hook loader abstraction and maybe a Plugin Factory and some interfaces for some common behaviors.

Let's keep in mind that we don't want to overcomplicate this starter generator and maybe some things can be implemented as dependencies.

Here's a plugin implementation approach with some interesting ideas: https://schlessera.github.io/wcnmgn-2017/

[csrui]

Sorry for the AFK but I've been on vacation. I meant the lack of other devs commenting on this thread. Sorry, tackling many things at the same time.

Like I said I'd love to hear some feedback from all log devs about the current state of our "plugin framework" (not the generator but what it generates).

I agree with you 100% and I'd say it is my main goal, to build it as a reusable dependency.

• Use composer to make the code available
• Heavily documented to help other devs.
• It should greatly improve DRY avoiding the current code / file repetition on every plugin.
• Automate as much as possible without sacrificing customisation.
• Contain unit tests
• Reduce the yeoman generator's output.

Share your thoughts (thanks for the link, that guy has some good reads).

[RicCastelhano]

@csrui sorry mate. I dont know why but I dont receive notifications for this repo. Let me catch up with this subject.

I'll follow up with another question. Why pass Plugin as a parameter of the class PostType? Is it just for the slug or some other thing? I did some digging in older plugins and it seems soo. For instance, in an on-development project of ours, we removed that parameter.

I'd ❤️ to hear the pain points of each dev with the current one but have no ideia how to collect them.

Unfortunately we don't have many internal dev's working on OSS matters.

While no feedback was offered, I just wanted to let you know that a new / reworked "plugin framework" is on the works to be presented maybe early next year.

That's great. Please make sure to present it to us and if it is top-notch, we should build a Yeoman generator for that structure. A generator is a better approach to prevent human-mistakes and, more important, the usage of the correct plugin framework by junior devs. I want to eliminate "copy/paste" to a minimum and promote automation to a maximum. Yeoman allow sub-generators, so we may have the initial plugin structure as the main task and all the additional features (CPT, TAX, AJAX, ...) as sub-tasks. However it will not be possible to contemplate all the possible features. That said, and even if we have a scaffolder running on the team, I would like to promote a debate about some mandatory design patterns to a later include on the Standards. What say you ?

[csrui]

Well, for now I would ask... what would be a good log repository / plugin to look at? As in big, complex and diverse. This would be a good way to analise and extract patterns.

[Narayon]

Maybe the Replicast Plugin.
We're planning to open it.
Do you wanna help?

[csrui]

Ok, will look into that plugin.

Would love to help but honestly between the 11 hours a day for the Gulbenkian client and a two year old that never sleeps... Probably I wont be much help.

I can try but the rewrite we are discussing on this thread will be a big enough endeavour.