About two weeks ago, I published Menu Cache, a small WordPress plugin that lets you easily cache and re-use the rendered markup of your navigation menus, and avoid (unnecessary) costly database queries and computations. Surprisingly, the GitHub repository has been starred more than 25 times within the first week; and all the marketing was just a short tweet.
Fellow WordPress engineer Andrey āRarstā Savchenko, however, responded like so:
we need to start labeling these with number… š
— Andrey Savchenko (@Rarst) October 25, 2016
At first, I didnāt get what this was all about. Well, it was all about the fact that my Menu Cache was not the first plugin that lets you do what it is designed to do. There is, for example, Menu Cache by Ryan Hellyer, or WP Nav Menu Cache, to only name a few. So Rarst was right. Well, of course, he was: long time ago, he published a plugin called Fragment Cache that, besides other things, is able to perform menu caching.
Anyway, the tweet made me wonder why I didnāt do a proper search before coding the plugin from scratch. Working as a WordPress engineer at Inpsydeāand as a developer in generalāI usually donāt just right away tackle the problem at hand, but rather try to search for an existing solution that I can either use as is, or maybe first customize or improve it in order to make it suit my specific needs.
Why had it been different that day?
Understanding
I did some thinking to understand the reason for having developed Menu Cache without having searched for existing plugins first. To repeat that: I did not run a search for this. Not a single one.
To be honest, the plugin is fairly simple. It only adds two specific filter functions and comes with a few filters itself so you can customize how the caching works. But still, I should (and normally would) have done some research first. So why didnāt I do it?
Finally, I thought about the last third-party plugin that I had to deal with. And by now Iām pretty sure: that very plugin was the reason I subconsciously moved away all thoughts about the possibility that there might be a (good) plugin out there that could do the job. Retrospecting, itās more than sad that a plugin can have such a (negative) influence, so Iām writing this post right now.
The Worst Maintenance Mode Plugin Ever
Fast-rewind yet another week, I was asked to review a premium plugin in order to make sure weāthis is Inpsydeācan safely use it as part of a client project. For this, we needed to be able to set up some pseudo-maintenance mode according to certain requirements. The back end should function just like always. When logged in, every user should be able to move in the front end like nothing happened. Certain IP( range)s should not be affected by the maintenance mode. Specific front-end URLs should be accessible for anyone. Other URLs should only be accessible for specific IPs. And a few conditions and exceptions more.
Like explained before, we did what we usually do: research. A colleague of mine found a plugin that claimed to be able to do anything we needed. It was a premium plugin, with a reasonable price. And the plugin had several positive reviews, too. So we bought a license.
Since we donāt just use some third-party component that we didnāt scutinize beforeāespecially for this particular task: temporarily keeping people outāI began reviewing the code.
Code Inspection
The first thing I did was to open the plugin folder in PhpStorm and run the built-in inspection on it. Well, OK. That gave me 433 weak warnings, 2,939 warnings, and 27 errors. What. The. Heck?!
But yeah, some of these (weak) warnings can safely be ignored, and lots of them were only due to missing or incorrect PHPDoc.
Searching for nonce found 82 results. Yay. š
A quick regex search for \$_(GET|POST|REQUEST|SERVER) got me 461 results.
Most of the times, using a specific value of a superglobal happened only after checking for its existence with isset() or empty(). Also, a lot of the usage was bound to some context check, for example, check_ajax_referer(). The values itself, however, almost never got sanitized in any way. š
Next, I regex-searched for [^!=](!|=)=[^=] to get an overview of loose comparisons: 1,089 occurrences. š
Beginning at the Beginning
After some Regex Fu for the whole project, I opened up the main plugin file. Lots of defines, a couple get_instance callbacks hooked to plugins_loaded, and mix ‘n’ match of indentations by a single tab as well as all possible variations of two through twelve (!) spaces. The number of subsequent blank lines ranged from zero to four.
The main plugin class was my next stop. Handy 818 lines short, it concerned itself with wiring up further components, performing direct SQL queries, populating several options, rendering settings pages, sending diverse HTTP headers, and doing lots of cache control stuff.
While the main plugin file had at least been consistent in the indentation for every single line, the main plugin class also perfectly illustrated how to perform back-and-forth switching of tabs and spaces within one and the same line. Pyutifull.
The Best Worst Part
The masterpiece surely is right away the constructor, so letās have it:
Since I do not want to publicly bash anyone here, I anonymized some identifiers in the original code.
What do we have here?
- fucked up indentation;
return false;ā¦ in a constructor;extracting imported data that might not even be what is expected;try {} catch ( $e ) {}ā¦ without reasonable exception handling (i.e.,catch);- manipulating a global object (e.g.,
$wpdb->suppress_errors = true;) without ever reverting the changes; - counterproductive complex code constructs (instead ofĀ
rtrim( ltrim( $foo, '/' ), '/' )dotrim( $foo, '/' )); - possibly redundand computations (e.g.,
if(((!empty($status) && $status === '1') || (!empty($status) && $status === '2') || (!empty($status) && $status === '3')))); - ā¦
Great! š And the code does not get any better. Ever.
All in all, I am unable to understand how this thing can work (for several people).
Summary
To make this clear: I am not saying that the plugin poses any effective dangers if used. I stopped mid-review, and we just abandoned (the idea of using) this plugin.
What I am saying, however, is this: that positively voted on, heavily used premium plugin is crap of very poor quality!
Quality and Price
It is no secret that there is a mutual relationship between price and quality. And there are at least two layers:
- For something of high quality it is OK to have a higher price. This is true for both the selling and the purchasing side.
- If something has a (slightly) higher price than something comparable the more expensive thing is often expected to be of higher quality as well.
Letās assume you want to buy some software. And letās assume you have two options: one program costs you 20 EUR, and another one costs 30 EUR, but it says ātestedā on its cover. Which one do you buy?
Well, of course, you buy the tested one. Just because itās not so much more expensive, and because using something that is tested makes you subconsciously feel better.
Usually this is absolutely OK. The problem is that such things can be used to fool you.
The same goes for other aspects. Some thing said to be premium can easily have a higher price than some equivalent non-premium thing.
We paid for something, and that something was said to be of good quality. Since it was absolutely not at all, we were utterly disappointed and annoyed.
Lessons Learned
Be willing to pay for something that saves yourself (a lot of) time.
Free open source software is great, no questions asked. But if you spent a lot of time working on something that might be of use for someone elseāand if you did a good jobāit is okay to put a price on it. Or ask for some reward in a different way (e.g., a donate button, a link to your Amazon wishlist, or in the form of postcardware, if you like postcards).
Donāt blindly trust (even lots of) reviews if you donāt know at least some of the reviewers.
Donāt blindly trust something just because it has a price.
Something Else?
Did I miss something important? Tell me.
It might be worth to mention that the idea of the menu cache plugin was born when facing the performance insanity of other poorly written premium plugins.
Once again, thank you for your insights, especially how you approach third party software.
It is always amazing to see an experienced developer dissect a plugin and comment on it’s weaknesses.
“return false; ā¦ in a constructor;” you say, and I nod, guessing it might not be clever to suppress error messages in a constructor. Yet I wonder, how an average WordPress user would know what to look for? What do people judge, if they write their reviews? Is it the code, they bought or are users already more than happy, if the documentation makes it easy to set up the tool and achieve, what was promised? (Not to even mention the puzzled users on the other side, who bought a premium theme and then start with an empty blog page, having no clue what to do.)
While I (hope to) get your point, but your conclusions still appear somewhat confusing to me: You suggest to pay for a software, but at the same time you also claim, that just paying money doesn’t guarantee you anything. But what would? Where do we find better vendors, that sell reliable products? Is the conclusion, that only well experienced developers should set up WordPress sites? What about WordPress’ claim, to “democratize publishing” then?
Often I have the impression, we keep telling people what not to do, instead of providing better choices. The result may be clueless users, who feel uncertain how to do better.
Whoa Bego!
When you comment on a post, one really has material for yet another post—if not more. š
I’ll try to clarify a few things, although some questions are better be answered separately—which I might be doing some day… š
Well, this was not about suppressing errors, but rather that it’s both pointless and suboptimal to return anything inside a PHP constructor.
When you create a new instance of a class—this is when the according constructor gets called, implicitly—the return value is the very instance.
When you explicitly call the constructor on an already created instance (i.e.,
$instance->__construct();), maybe because you want to unit test the constructor, you don’t really test the constructor at all, but only some logic inside. If the constructor sets properties, you do this twice. If the constructor calls other methods/functions, you do this twice. Maybe you trigger some error. Maybe the instance (or some other party, for example, some injected dependency) behaves differently on the second call than on the first one. And anyway, your constructors should not include (heavy) logic at all. Testing a constructor would be done as part of integration or system testing, and it would most probably only consist of calling the constructor, and then comparing the results of several getter calls with their individual expectations. Because that’s all what a constructor should do: set up the class properties.Well, whatever they want, I’d say. And that is fine by me.
Depending on whether you are a developer, a power user, or a beginner, and depending on whether you only want to use a plugin or theme as is or (have to) customize it, you might target different aspects of the product.
For the above maintenance plugin, my task was to make sure the plugin can safely be used for an enterprise client project. This means that I scrutinized the codebase, so this is what I reviewed.
Hm, this is not what I wanted to say. I rather wanted to convey two things.
First, don’t be too stingy to pay for something if you gain (anything!) by using it.
Second, don’t expect too much of something only because of claims.
These two things may be related, but they definitley stand for theirselves. And yes, the truth, in my opinion, is that doing both is sometimes (very) hard. And one can always be fooled still.
I believe there are very great producers of WordPress plugins and themes out there. No matter if free or paid.
But of course, you may not be able to find every special use case plugin (easily) by just doing a simple search.
It is true, however, that there are also lots of bad products in the wild. But this is something that we—this is you, me, and anyone—can (try to) make any better. Leave constructive (!) and detailed feedback or reviews. Or contact the author directly. Make people aware of the things you found (or didn’t find). This includes fellow developers, fellow users, and even more the authors.
Absolutely not.
Anyone should be able to set up a WordPress site. That is, if that someone spent enough time to read into anything necessary to do this. And WordPress makes this very much possible.
Depending on what you want to do with your WordPress site, though, you maybe should hire someone who does this on a regular basis. Like with almost any other trade, you hire some professional if you A) are not able to do something on your own, or B) you are not able to do this as good as a professional, but it is of importance that it gets done right.
If all you want to do is set up some personal blog, you might be OK to do this yourself. Maybe you still want to/should be using some premium theme and/or plugins (which also include support!).
If you plan to set up a multi-language multi-currency multi-region multi-vendor online shop with various (payment, review, shipping, etc.) integrations, you maybe should have someone else take care of this. š
This is not the claim of WordPress as software nor the community, but that of WordPress.com. And by being a customer of WordPress.com, one already is giving parts into the hands of professionals.
But maybe I just didn’t get what you wanted to say/ask with this…?
This is something that shouldn’t be the case. If someone is able to assess something is wrong that someone should at least by able to also give details about what is wrong and why this is, and maybe even provide a better alternative.
Another thing, however, is providing concrete solutions to abstract problems or situations. This is something that gets asked for a lot, but this is also something that is just not possible.
Tom Nowell recently published a great post on such a question.
When I read the title (on Twitter), at first I thought “Wait, what? This question cannot be answered in general.”. And if you read the post you know that this is exactly what it is all about. š
Thanks,
Thorsten
Thank you for your comprehensive answer, especially your explanation on return values of constructors.
Not trying to be a smart-ass š¤, I don’t agree with your answer regarding the claim “to democratize publishing”. That claim can be found as the mission statement on the website of the WordPress Foundation (“ā¦the mission of the WordPress open source project: to democratize publishing through Open Source, GPL software.”, http://wordpressfoundation.org/) and was declared as a mission statement for WordPress(.org), not Automattic, during Matt Mullenweg’s keynote “State of the Word” in 2014 (https://youtu.be/gfbT2dlhiFk?t=44m13s).
The reason I refer to it, was the blog post by Andrey “Rarst” Savchenko about WordPress crusade against technical responsibility. Andrey questions in that post, whether this claim may be (mis-)understood as ” Democratize publishing by empowering people to create sites quickly, easily, and without a clue.” You should “better be good, at what you do,” Andrey writes, as “few good things ever happened by accident and ignorance”.
In the past year I have talked to a bunch of people who admit to be an absolute beginner and have been using WordPress for a couple of weeks at most, but at the same time feel empowered to run a web shop, as WooCommerce can be set up so easily. What could go wrong? These users don’t mind, what could happen to their client’s (payment) data, if their site would get hacked, nor do they care much about consequences, if they “forget” to add even the most simple legal informations.
My point is, that I doubt, these users are skilled enough to “scrutinize the codebase” of third party software as carefully and with as much knowledge, as described in your post. I certainly agree with you, that these users “should have someone else take care of this” ā just without the “maybe”.
Well, sometimes I should look things up somewhere else than (just) in my head. š
Thanks for teaching me one of the WordPress(.org) mission statements. š
I do believe, however, that there has been a recent discussion about exactly that… Whose claim it is and what it should mean exactly.
Hey Thorsten,
For enterprise use, there’s only a very small subset of plugins that makes sense to use as is. These are large, mature plugins where doing a custom coded solution would just take to much development and testing time. In such cases, choosing bad code quality trumps delaying the launch for 6 months or more.
But overall, I find that plugin code quality is pretty bad across the board. There are exceptions, of course, but the large majority of the plugins, whether they are premium or free ones, is sloppily crafted and using a mixture of legacy and modern PHP while mixing purely procedural code with pseudo-OOP syntax.
I can totally understand where Bego is coming from, and this is a huge issue for the WordPress ecosystem. Plugins and themes cannot be relied upon, and often cause all sorts of issues for anything but the most trivial sites. But this shouldn’t be the end user’s problem… How is a user supposed to know what to use and what to avoid?
The best way to improve this situation is to improve general development knowledge by educating and sharing code and best practices. Developers improve through external feedback and peer reviews. However, I found that especially WordPress developers often tend to be very defensive about their code. When someone provides constructive criticism, it is sometimes perceived as an offense, rather than an opportunity to improve, unfortunately. This is handled in a much more productive way in some of the other open source communities I have been allowed to be a part of.
I’m not sure how this situation could be better handled for end-users right now, to be honest. There’s a lot of good information being shared about how to do it in a better way, but the sheer volume of bad advice makes it hard to distil the signal from the noise. We need a cultural shift as much as we need a better set of tutorials.
And, for what it’s worth, the above screenshot of the constructor does indeed combine so many problems in one single method that it sends shivers down my spine…