# Pluggin' in Plugins

I'm currently working on an in-house project and a part of that project is this pretty standard plugin loader below.

There must be a way to avoid so many ugly looking else statements.

I'm kinda hitting a brick wall with getting better at "writing less to achieve the same thing".

How I can improve the code as I do think it's pretty clean, but I got this aching feeling that I've been missing a lot of "shorthanding" my code?

/**
* Load plugins specified by the array $load, containing the leading filenames in the /src/plugins/ folder * All plugins must be proceeded with *.plugin.php where the asterisk is the name of the respective plugin * * @since 0.1 * @param array$load e.g: array("Users", "Products", "Pages")
* @return void
*/
public function __construct($load) {$this->plugin_dir = $_SERVER['DOCUMENT_ROOT'] . "/PlugCMS/src/plugins"; require_once($this->plugin_dir . "/Users.default.php"); // Included by default, and is required to gain access to the dashboard
require_once($this->plugin_dir . "/Pages.default.php"); // You can remove this but you will need to remove the pageLoader.php mod_rewrite rule from .htaccess to get control of the front-end back. if (is_dir($this->plugin_dir)) {
if (is_array($load)) {$count = count($load); for ($i=0; $i <$count; $i++) {$filename = $this->plugin_dir . "/" .$load[$i] . ".plugin.php"; if (file_exists($filename)) {

if (@require_once($filename)) {$this->load_output['active'][] = $load[$i];
}
else
{
$this->load_output['failed'][] =$load[$i]; } } else {$this->load_output['failed'][] = $load[$i];
}
}

}
else
{
$this->load_output['error'][] = "Error:$load must be an array";
}
}
else
{
$this->load_output['error'][] = "PlugCMS has detected the default plugin directory is missing ({$this->plugin_dir}). Please check the installation and try again..";
}

}

• Argh... seeing code that contains @require_once really makes my blood boil. Please don't use the @ operator of death: if you see a warning/notice/error, FIX IT, don't silence it. – Elias Van Ootegem Oct 23 '14 at 19:20
• @Elias The default plugins stop on error, the user-defined ones are silences as it's not worth it causing the site to halt just because someone messed up a plugin, they can debug outside of the plugin loader. – zanderwar Oct 23 '14 at 23:15
• I have changed it to @include_once too as suggested by @tim and @janos – zanderwar Oct 23 '14 at 23:25
• That's not the point. The point is your use of @. If an include produces an error/warning/notice, then you should work out why it does so, and fix the root cause. Supressing the error doesn't fix the problem. Think of it as cleaning your house: you can wipe the dirt under the carpet so you don't see it, but it's still there; or you can get a vacuum cleaner and remove it entirely. The latter is the cleaner option. – Elias Van Ootegem Oct 24 '14 at 6:32

I think surely there must be a way to avoid so many ugly looking else statements.

Return early, for example:

if (is_dir($this->plugin_dir)) {$this->load_output['error'][] = "PlugCMS has detected the default plugin directory is missing ({$this->plugin_dir}). Please check the installation and try again.."; return; }  You can do the same for is_array($load), and already you got rid of two levels of nesting.

You can also combine file_exists($filename) and @require_once($filename) to file_exists($filename) && @require_once($filename) to get rid of another level of nesting.

• Thank you for your reply, I'm aware of return early but the opportunity is unfortunately quite rare, touché on combining the file_exists() and require_once(). Would anyone have a reference URL with something similar to add to what Tim said; but not limited to? I've come up NULL on anything decent throughout my searches. – zanderwar Oct 23 '14 at 13:23
• you have an extra Curly Brace in there?? – Malachi Oct 23 '14 at 14:49
• @Zanderwar try searching for reduce deeply nested if statements (no reason to include a language, the principles are pretty much universal), there are quite a few results for it. – tim Oct 23 '14 at 17:02

I'm aware of return early but the opportunity is unfortunately quite rare

Two levels removed as @tim's suggestions, it's quite significant. With his other suggestions applied too, the whole thing becomes quite a bit shorter:

if (!is_dir($this->plugin_dir)) {$this->load_output['error'][] = "PlugCMS has detected the default plugin directory is missing ({$this->plugin_dir}). Please check the installation and try again.."; return; } if (!is_array($load)) {
$this->load_output['error'][] = "Error:$load must be an array";
return;
}

$count = count($load);

for ($i = 0;$i < $count;$i++) {
$filename =$this->plugin_dir . "/" . $load[$i] . ".plugin.php";

if (file_exists($filename) && @require_once($filename)) {
$this->load_output['active'][] =$load[$i]; } else {$this->load_output['failed'][] = $load[$i];
}
}


In addition, I think you can replace this:

if (file_exists($filename) && @require_once($filename)) {


With this:

if (@include_once($filename)) {  Because if the file doesn't exist, the include_once will fail anyway. Originally I suggested @require_once there, but that would be bad, as @tim has pointed out: As require_once will halt the script on failure, only using if (@require_once($filename)) will not work (the script will stop completely, and the else statement will not be executed). So either the file_exists should be kept, or include_once should be used (this probably makes more sense, but might be a bit harder to read).

I know this is old, but I feel it important to note that include, require and their respective *_once statements are language constructs, not functions. As such, you should stylistically avoid using brackets:

require_once \$this->plugin_dir . "/Users.default.php";


It will work with them, but it's considered bad practice to use them.