patternphpMinor
My first project using namespaces
Viewed 0 times
projectfirstusingnamespaces
Problem
I have to create some APIs for a online game and I decided to learn to use namespaces correctly.
I used the factory pattern, then the
The folders are synchronized with the namespaces.
The main class
Autoloader class:
An example of a module:
Everything works well, but I want to know if there are any things I did wrong or if there are things I can do better.
I used the factory pattern, then the
loadModule method checks if in the Modules folder there is a class named $module and then it return the object.The folders are synchronized with the namespaces.
The main class
Metin2.class.php:db = new Utilities\Database($database);
} catch (Exceptions\Metin2Exception $e) {
echo $e->errorMessage();
}
}
public function loadModule($module)
{
$m = "\\Metin2\\Modules\\" . $module;
return new $m;
}
}Autoloader class:
<?php
namespace Metin2\Utilities;
class Autoloader {
public static function loadModel($class){
$class = str_replace("\\","/",$class);
$class = explode("/",$class);
if($class[0] == "Metin2")unset($class[0]);
$class = implode("/",$class);
$path = ROOT_PATH.$class.".class.php";
if(file_exists($path)){
require_once($path);
}
}
}An example of a module:
db = \Metin2\Utilities\Database::getInstance();
print_r($this->db->getOne("account"));
}
}Everything works well, but I want to know if there are any things I did wrong or if there are things I can do better.
Solution
My thoughts:
-
Imho
-
Is there a reason to keep all your application in
-
-
It's worth considering using specialized Exception classes and keep Exception messages for details. That is because you can't catch Exception based on its message, but only class. Here you could have
-
Instead of echoing Exception message I would recommend making a function/class, that would enable you to redirect error messages to file, as it's not recommended to be printing out application messages for users to see. This way you'll also be able to log them.
-
On a similar note you may take a look at
-
You can remove dependency of
-
When
-
What's the purpose of
-
Imho
Database should not belong to Utilities namespace. Working with database is likely a core part of your application, "Utilities" suggest auxiliary tools to make your life easier. But maybe the class does something different than I expect-
Is there a reason to keep all your application in
Metin2 namespace but have separate Exceptions namespace with Metin2Exception? If it's for code completition, then a good IDE should only suggest classes inheriting from Exception in these contexts.-
Metin2::__constructor(): can Utilities\Database throw Metin2Exception? If not, then it's not really useful to throw exception only to catch it right away and echo-
It's worth considering using specialized Exception classes and keep Exception messages for details. That is because you can't catch Exception based on its message, but only class. Here you could have
ConnectionFailedException extends Metin2Exception.-
Instead of echoing Exception message I would recommend making a function/class, that would enable you to redirect error messages to file, as it's not recommended to be printing out application messages for users to see. This way you'll also be able to log them.
-
On a similar note you may take a look at
set_exception_handler, that lets you assign a function/method to handle uncaught exceptions. And from that you can try either making your own or using a 3rd party exception handlers, which combined with set_error_handler that can be used for conversion of errors to exceptions can really improve your programming. Examples: https://github.com/nette/tracy or ... maybe http://raveren.github.io/kint/ (I'm sure there's another as nice as Tracy, but I can't remember its name).-
You can remove dependency of
Autoloader on a global constant by adding a static property and using a setter to set its value-
When
Autoloader finds out the target filepath does not exist, it does not react in any way ... maybe at least log it somehow, if not throw exception?-
What's the purpose of
Testdb? I would discourage from using a service location especially when it comes to testing ... why not pass the $db as an argument?Context
StackExchange Code Review Q#44565, answer score: 5
Revisions (0)
No revisions yet.