I created a class that I plan on using to autoload my project classes (20-30 classes) maximum. I wonder if there is anything, anything at all, that I can improve on this class to improve it any further.
Please let me know your thoughts below.
<?php
/**
* YodaCMS, content management system.
*
* @info This class autoloads project classes.
* @author Someone Someone <[email protected]>
* @version 0.0.1
* @package YodaCMS
*
*/
class autoLoader {
private static $loadedLibrarys;
public function __construct() {
$loadedLibrarys = array();
}
public static function loadLibrarys() {
newLibrary("testClass", new testClass());
}
private function newLibrary($lName, $lValue) {
if (!$this->tryLoadLibrary($lName, $lValue)) {
print_r('Error loading library: ' . $lName);
}
}
public function tryLoadLibrary($lName, $lValue) {
include(ROOT . '/applocation/base/classes/'.$lName.'.class.php');
$this->loadedLibrarys[$lName] = $lValue;
return true;
}
}
-
\$\begingroup\$ Any reason why you don't use the PSR-4 autoloader? php-fig.org/psr/psr-4 ? \$\endgroup\$Pinoniq– Pinoniq2016年04月03日 23:13:18 +00:00Commented Apr 3, 2016 at 23:13
1 Answer 1
I don't think that you need newLibrary
and tryLoadLibrary
. The names don't really make clear what the distinction is, and I can't imagine that you'd ever want to load a library but not know if it failed or not.
Also, instead of printing, I would throw an exception. That way, the calling code can handle the case (what if it's an irrelevant library? Now the user is still annoyed with a meaningless message; but what if it's a highly relevant library? Is a small message really enough?).
I would also get rid of the distinction between lName
and lValue
. If you follow conventions, they should be the same anyways. If they are not, it might get confusing, especially because it's not just this call, but also how they are stored in loadedLibrarys[$lName] = $lValue;
,
I would also rename tryLoadLibrary
to loadAdditionalLibrary
. try
is not really needed (we can assume that it will only try), and additional
makes it more fitting with loadLibrarys
. lName
and lValue
are also not all that clear; if I didn't read your code, I'd have no idea what l
is, or what name
and what a value
is. libraryClassName
and libraryFileName
would be clearer (you can also skip the library
part, as it's clear from context.
Finally, I would always check for directory traversal when including with a variable. It shouldn't really be necessary in this situation, but it's just good practice, and doesn't cost that much.