Anyone know good coding when they see it?

[internetheaven]

I have had a string of bad programmers and was hoping someone could take a look at the code below and tell me what they think of the coder's abilities. I am not a programmer, I can make general edits to PHP programs that have alread been written but that's about it so I rely heavily on outsourced programmers. So, if anyone could tell me what they think of this I would appreciate it:

<?php

include_once(MODULE_CLASSES . "AbstractModule.php");

class CModulesController
{
const PHP_EXTENSION = ".php";
const TPL_EXTENSION = ".tpl";
const CLASS_PREFIX = "C";

protected static $instance = null;
/**
* Modules collection
*
* @var array
*/
protected $modules = array();

protected function __construct()
{
$this->setModules($this->getNames());
}

/**
* @return CModulesController
*/
public static function getInstance()
{
if (self::$instance == null)
{
self::$instance = new CModulesController();
}
return self::$instance;
}

/**
* @param array $modules
*/
protected function setModules($modules)
{
$this->modules = $modules;
}

/**
* @return array
*/
protected function getNames()
{
$names = array();

$handle = opendir(MODULES_PATH);

while (true)
{
$file = readdir($handle);

if(!$file) break;

if($file!= "." && $file!= ".." &&!is_dir(MODULES_PATH . $file) && strpos($file, self::PHP_EXTENSION))
{
$name = str_replace(self::PHP_EXTENSION, "", $file);
$names[$name] = null;
}
}
closedir($handle);
return $names;
}

/**
* @param string $name
* @return bool
*/
public function isModule($name)
{
return array_key_exists($name, $this->modules);
}

/**
* @param string $moduleName
* @param string $action
* @param array $params
*/
public function getContent($moduleName, $action, $params=array())
{
if (!$this->isModule($moduleName)) throw new Exception("No such module: " . $moduleName);

$module = $this->getModule($moduleName);

$data = $module->getData($action, $params);

return $this->parseData($data, $moduleName, $action . self::TPL_EXTENSION );
}

/**
* Get module by name
*
* @param string $moduleName
* @return CAbstractModule
*/
protected function getModule($moduleName)
{
if ($this->modules[$moduleName] == null)
{
include_once(MODULES_PATH . $moduleName . self::PHP_EXTENSION);
$className = self::CLASS_PREFIX . $moduleName;
$this->modules[$moduleName] = new $className();
}
return $this->modules[$moduleName];
}

/**
* Parse data to HTML
*
* @param array $data
* @param string $templateName
* @return string
*/
protected function parseData($data, $moduleName, $templateName)
{
$app = CApp::getInstance();
$app->template_vars += $data;

$file = MODULES_TEMPLATES_PATH . $moduleName . "/" . $templateName;

return CTemplate::parse_file ($file);
}

/**
* @param string $moduleName
* @param string $operation
* @param array $params
* @return mixed
*/
public function doOperation($moduleName, $operation, $params)
{
return $this->getModule($moduleName)->{$operation}($params);
}

/**
* @param string $moduleName
* @param string $operation
* @return bool
*/
public function hasOperation($moduleName, $operation)
{
$module = $this->getModule($moduleName);
return method_exists($module, $operation);
}

}

?>

Is it advanced? Is it something standard that this firm would not even have had to write themselves? Is it the long way round to a simple problem? Any comment would be great.

Thanks
Mike

[RonPK]

Is it advanced?

Well, it is object oriented code, which is not something most beginners are capable of. The big advantage of OO code is it's reuseability, i.e. if done properly it can be used under different circumstances without any modification to the code.

Here are the results of the RonPK jury: ten points, <span lang="fr">dix points</fr>. Good: OO, no content or HTML in the code, some documentation, distinction between public and protected methods, PHP 5. Bad: the documentation could be more verbose.

Is it something standard that this firm would not even have had to write themselves?

It seems to be a core part of a modular CMS templating system. There is many of them around. The firm may argue that writing your own code has the advantage that you're absolutely sure about how it works and what it does.

Is it the long way round to a simple problem?

Because of the level of abstraction it may look complicated, but abstraction should increase reusability.

[internetheaven]

Thanks for the input:

Bad: the documentation could be more verbose.

Could you please elaborate?

Thanks
Mike

[henry0]

Mention clearly (almost plain English) by segment what each segment does.
so if the coder comes back to the script after months
he/she will have a clear pic of the logic

same applies if a new coder embarks on the project.

It saves a great deal of time and is considered as a good coding principle

[willybfriendly]

Code your comments rather than commenting your code!