Hi Vince,
One question: why not use the wpp_get_mostpopular() template tag with the post_html parameter instead? (see ‘How can I use my own HTML markup with your plugin?’ on the FAQ section).
Hi Héctor,
why not use the wpp_get_mostpopular() template tag with the post_html parameter instead?
I sorry that I cannot say a lot about this project, but I’ll try to explain the situation for you.
So, as mentioned earlier, the posts have a lots of special, structured extra data. These data have to display for the users. I tried to write the post_html parameter, but with the placeholders, defined in WordPressPopularPosts::format_content() method, no way to show the special post meta fields, or other not handled data.
In earlier (2.3.2) version I extend WordPressPopularPosts class, I have to unregister your widget class and register my (overwritten) class to make it work. In my class I override get_popular_posts function, and call the original one with $return = true paramter. I got the posts data from your method, so i could write my own output. But in version 2.3.3 this method ignore the return parameter and returns all the formatted content. I do not like this way to create special formatted post output, because I built it on a non documented behaviour and i don’t think the widget replacement is a good long-term solution.
I think its a good think to use actions and filters in plugin development, so theme and plugin developers could add extra functions and features without hacking original code
That was actually the reason why the $return parameter existed until 2.3.2, but since it seemed that no one was using it (not even myself) I removed it on v2.3.3.
In your opinion, what would be a better solution?
I think in WordPress ecosystem a plugin should define and use filters and actions.
I think in active WordPress development it’s much easier to provide backward compatibility with filters than with custom classes.
Well, to be honest I’m not very familiar with the use of custom actions/filters but will look into it. Learning new stuff is always a good thing, so I’ll give it a try!
Thanks for the suggestion!
Actually, it was easier than I expected. I just implemented a new filter for the get_popular_posts() method that provides access to the popular posts data array that you can use to build your own HTML markup.
Usage:
/*
* @param string $html: original HTML output.
* @param array $data: array of objects containing
* the popular posts information.
*/
function my_custom_html( $html, $data ){
echo "<pre>"; print_r( $data ); echo "</pre>";
// do whatever you want with $data here
// and return your custom HTML code when you're done
die(); // I believe this bit isn't really necessary
}
add_filter( 'wpp_html', 'my_custom_html', 10, 2 );
This approach is much cleaner and keeps plugin’s code untouched. No need for hacks. What do you think?
If you want to test it, download the development version of the WPP plugin here.
It looks fine. My patch define a filter after the DB query and before the HTML markup builder codes. So when some plugin implement that filter, the default HTML build routine unnecessary to run. Because someone is want to override the default output.
I think the best we could do is define multiple filters.
- Before HTML build, as in my first patch. With this we do not need default HTML builder behaviour (call it
wpp_custom_html)
- In the end of the
foreach($mostpopular as $p) loop for every post output (wpp_post)
- And a last one for the end, as you just defined
wpp_html
=======================
If you don’t mind i give some advice. There are some principles I follow when write codes. A functions should do one and only one thing. When I keep my functions and methods as simple as possible at the end I get easily read and understandable code. Only two of these: KISS principle and DRY principle. You can read about these principles, and more in Clean Code by Robert C. Martin.
Now the WordPressPopularPosts::get_popular_posts() method is doing multiple things:
- Builds an SQL query
- Run the builded SQL query
- Iterates over the results
- In this loop builds a default HTML post output…
- … but if the user set some custom
post_html builds post HTML from that layout
With an intensive refactoring the code of WordPressPopularPosts can become extremely readable and much more developer friendly. If you have a clean code you can implement new features faster and safely.
If you think and don’t mind i can help to refactor your code. What do you think?
Sure, please go ahead! Any suggestions for improvements will be welcome!
It’s on Github: https://github.com/tikaszvince/wordpress-popular-posts. I did not finished the refactoring, but you could see the commits step by step whats happening.
Nice work, Vince! I understand most of the stuff you did there, except for this commit. What is $cache for?
That is a little optimization.
When I created the _get_title() method I realized this new function will be called two times on every post rendering. It will be called to create the $title variable in render_popular_post function and it will be called again in _get_thumb().
The _get_title() can be resource-intensive method if qtrans is presented and other plugins/themes are defined some filter on the_title. So we can save some time and resource if we use the previous result.
This cache is only available for runtime. It uses a static variable for store datas.
I had some free time so I continued the refactoring: https://github.com/tikaszvince/wordpress-popular-posts/commits/master. There are some @todo comment. These notes mark some problematic points:
- First of all, I think the admin menu and administration panel setup should not be in the widget constroctors.
- wpp_upgrade() have to move into include/update.php
- I think we need a standalone query builder class.
- image_resize() is deprecated
- I think in get_img() there is a code block could not run
- and there are huge if/elseif/else structures that make it difficult for understanding the logick
Hi Vince,
Thanks for the input! I’ve been following the changes on your github account. Left a comment there about image_resize and the code you mentioned.
Also, seeing your work made me want to rework the plugin – so I took most of your suggested changes and merged them into a newer version I’ve been working on. I’ll be probably uploading this version to Github as well, maybe this weekend, so I’m probably going to merge any new commits you’ve made that I may have missed.
If I replace my wpp folder with your patched one on github, how can I then insert a custom output into my index.php?
I only need range, limit, and for it to follow a content-pop.php.
Thanks!
My recommendation would be to wait until the fix is complete.
If you really want to try, download this version of the WPP plugin and follow these instructions. Filter name may change in the future, though, since this is still a development version.
By the way, I’ve finally moved the plugin to GitHub. It’s updated until version 2.3.5 (current official release), and I’m already commiting changes and bug fixes there. There’s a more recent version of WPP based on Vince’s code refactoring, but it won’t be available yet since I want to release a stable 2.3.6 before jumping to 3.0.0.
