WordPress.org

Ready to get started?Download WordPress

Forums

Business Directory Plugin
For Developers: Great Support, It Works; however, not developer friendly code (7 posts)

4 stars
  1. Bob Gregor
    Member
    Posted 3 months ago #

    make sure you view the entire thread - great support from the plugin author, committed to customer service. That's huge in my book!
    I reviewed the code structure of this plugin. The code is not commented and lacks any docblocks that I could find in the 15+ files I reviewed. The code is also riddled with globals, which wouldn't be a huge problem if the code was commented properly, which would allow any modern IDE to follow the inheritance/types of the code.

    There are also bugs that haven't been tested or caught, such as using undefined variables. For example, in (in plugin 3.6) the file /views/views.php line 103 has an undefined variable $msg that's being passed by reference, it should be initialized on line 102 as $msg = ''. Again, this would be helped by proper comments.

    It also lacks the ability to use the WordPress API to add actions or filter content.
    In the templates directory, there are 7 do_action calls: http://cl.circletr.ee/image/3u1y3L3E0L0o
    Compare that to WooCommerce's 158: http://cl.circletr.ee/image/370X2I2J340d
    --For example, it's very useful in WooCommerce to be able to add an action to a checkout page, and not have to copy/override an entire template. Why? Because when WooCommerce releases the next major release, it's very easy for them to keep a do_action() call in a template, and then restructure the entire page's DOM, without fear of breaking interoperability.

  2. businessdirectoryplugin
    Member
    Plugin Author

    Posted 3 months ago #

    Hi Bob,

    Thanks for your detailed review here. The vast majority of our users don't really look into the internals of the code in the way you did.

    We'll check for the bugs that you found above--I'm curious as to how you located those syntax errors. If there's a tool we can leverage to improve our code, I'm all ears.

    We are always adding additional hooks and filters as our clients request and need them. While comparing us to WooCommerce make seem relevant, they also have a staff of 20 dedicated developers where we have but 1, so they clearly have more resources to dedicate to their plugins.

    We're always working to improve, but the internals of the plugin may not be "perfect in every way". What does matter to our customers is how the directory functions, what drop-ins can be added quickly without much customization or fuss, and I think you can see that most of our clients do find this plugin meets or exceeds their needs in that area.

    If you feel like there are some action hooks we can add that would greatly improve your ability to customize, I certainly welcome those suggestions and we can get them in sooner rather than later.

    Please feel free to contact me directly if you want to continue the conversation further: http://businessdirectoryplugin.com/contact/

  3. Jim Hall
    Member
    Posted 3 months ago #

    Hi there,

    He probably enabled debugging in the wp-config file to find the undefined variables. Between that and Firebug, errors can't hide.

    I too like to look over the code like this for higher-profile client sites before going live.

  4. businessdirectoryplugin
    Member
    Plugin Author

    Posted 3 months ago #

    Hi guys,

    After reviewing some things with my developer, the variable issues you raised above have been addressed, although it's largely a matter of style since it's a variable passed by reference and initialization isn't required in such cases when you plan to return values from them (as we do in our code). Some other common examples of this practice include parse_str() from PHP itself. If you scroll down to the examples section of the code, you'll see the function is called with an $output argument without $output being initialized anywere. The same happens in preg_match, preg_match_all and a lot of other PHP functions. That's all we're doing here.

    We didn't see any obvious errors being ignored even with WP_DEBUG turned on and looking in Chrome developer console or Firebug (both). If you see something specific that we might be missing, please let me know.

    As for the globals issues, I'm not sure what plugin variables you might be referring to--we have but one single global variable: $wpbdp, the Singleton that accesses the core class. All other classes we use encapsulate the variables they own in a reasonable way. If you found something else, please let me know, but my research didn't find what you mentioned above.

    Also, as for documentation on the API, we have this on our roadmap to open up the API. Clearly WooCommerce has a published API and therefore, must have hooks and filters for just about everything. We will look to add this to our API as we migrate the plugin code. A good deal of our new features have the docblocks embedded in them, but it's true that not all the plugin (which is 229 files and 31K lines of code) has such documentation as we prioritize the feature development and bug fixing over the beautification of the code, since the customer use is our #1 priority.

  5. Bob Gregor
    Member
    Posted 3 months ago #

    I was using PHPStorm 7.1.3 - which runs a variety of runtime checks on any open files, and can run even more in-depth code analysis (which I did not).

    Great response & engagement +1 star for that ;-)

    As far as hooks - I totally understand the 1 staff vs. 20 approach. It's my preferred method. However - I'd encourage your next release to think about a few use cases for your product, and how you'd extend it as a 3rd party developer. That way, there will be filters & actions that allow us to extend it in a way you've planned for, and we don't have to worry about breaking future compatibility.

    As for globals - I guess technically speaking - you are correct, in terms of inheritance. However, if you look at file /api/api.php and see how the wpbdp() function returns a global variable without a doc block, and then the corresponding methods reference said global (& it's inherited classes / methods), it loses the IDE compatibility.

    For example, in api.php, adding the following docblock will help the IDE figure out variable types, and enable code completion / hot linking to other classes / methods

    /**
     * Gets application interface
     * @return WPBDP_Plugin $wpbdp
     */
    function wpbdp() {
        global $wpbdp;
        return $wpbdp;
    }
  6. businessdirectoryplugin
    Member
    Plugin Author

    Posted 3 months ago #

    Hi Bob,

    Thanks for the response--can you elaborate on the "use cases" you mention above for the hooks, if you have some specifics in mind? It's more important that we capture how YOU want to use it than how we think you might. :) We're in the process of documenting that API use and addition of hooks right now, so this is a good time to hone that.

    I will make sure my developer is aware of your suggestions above and we'll incorporate them ASAP.

  7. Bob Gregor
    Member
    Posted 3 months ago #

    Thanks!

    One of them is a property listing site for Vacation Destinations - for example, rentals in Maine, and Vacation destinations in Florida.

    Also - just some general UX/Sales/Impressions/feedback on your website:

    1. Add a lightbox plugin for media links: http://cl.circletr.ee/image/223h292z0d2u (Clicking an image, and opening a direct link isn't the most friendly)
    2. The way the demo is shown, doesn't give me confidence in the plugin, for example, this page has broken floats: http://cl.circletr.ee/image/0e0a02170K3U (Tested in Chrome 34.0.1847.131)
    3. Another example, this page has hidden overflow content: http://cl.circletr.ee/image/212T2l2A1r0Y

    Is there an admin demo of the plugin? I'd also recommend moving the demo plugin to a new subdomain/subdirectory, with a twenty* WordPress theme - that way we can see what the plugin does vs. your main site's design. You can also have a cron that resets the database every 60 minutes, for example

Reply

You must log in to post.

About this Plugin

About this Topic

Tags

No tags yet.