WebDevStudios / oops-wp

A collection of abstract classes, interfaces, and traits to promote object-oriented programming practices in WordPress.
57 stars 9 forks source link

Renderable render methods don't render, ambiguous method #19

Closed tomjn closed 5 years ago

tomjn commented 5 years ago

Rendering should render the result, instead it appears to return it as a string. This is misleading and implies that a renderable is responsible for output, and by conjunction late escaping, however the use of renderable for shortcodes implies the opposite is true.

It's not clear due to inconsistent use, if render outputs or not. If it doesn't output, but returns, then it shouldn't be called render as it doesn't render to anything

A rename would be the best solution, I would suggest html(), getHtml(), content(), or getContent(). The versions with get are more descriptive, but type hinting either via PHP or the docblocks would also be a big improvement

jmichaelward commented 5 years ago

Hi @tomjn! I apologize for the extremely late response to this ticket – I unfortunately hadn't been notified of its presence and am just getting back over here this month for 5 for the Future at WDS. I appreciate you taking the time to open this issue!

Some clarification about the Renderable interface - there is no return value defined, as the expectation is that any object that needs to implement this interface should be displaying some kind of value. The implementation of that interface is up to the developer, but the expectation is that render will display to the screen. All concrete implementations should provide their own logic for that render, which generally might be (but is not limited to) some kind of echo statement in combination with an escaped value.

Currently, the only OOPS-WP structure that implements Renderable is the Shortcode object, by way of the defined ShortcodeInterface. The implementation of render is again left to the engineer, but since all shortcodes generate output to the screen, objects that extend Shortcode must also define the logic for how that is done.

Other use cases for the render method might exist outside of default WordPress structures. Any object in your project can implement this interface to declare a contract for providing a method that will generate screen output.

Please let me know whether this satisfies you question or if you need additional clarification. I realize the inline documentation of the Renderable interface is somewhat sparse, so if there are clarifications you feel would be helpful for other engineers, I would greatly appreciate hearing them.

Thanks again.

tomjn commented 5 years ago

Output buffers can be used for the purposes of shortcodes, but I agree, those methods should output directly, rather than using return variables

jmichaelward commented 5 years ago

Great. Thanks, @tomjn. I'm going to this as wontfix at this point in time, since whether to implement render is the choice of the implementing developer.