1

我正在寻找关于我的控制架构脚本的反馈(包括在下面)。具体来说,我正在寻找有关脚本设计、组织、评论和格式的反馈。我喜欢 php 编程作为一种爱好,并且正在寻找可以改进我的代码的地方。

提前致谢!

class FrontController extends ActionController {

//Declaring variable(s)
private static $instance;
protected $controller;

//Class construct method
public function __construct() {}

//Starts new instance of this class with a singleton pattern
public static function getInstance() {
    if(!self::$instance) {
        self::$instance = new self();
    }
    return self::$instance;
}

public function dispatch($throwExceptions = false) {

    /* Checks for the GET variables $module and $action, and, if present,
     * strips them down with a regular expression function with a white
     * list of allowed characters, removing anything that is not a letter,
     * number, underscore or hyphen.
     */
    $regex  = '/[^-_A-z0-9]+/';
    $module = isset($_GET['module']) ? preg_replace($regex, '', $_GET['module']) : 'home';
    $action = isset($_GET['action']) ? preg_replace($regex, '', $_GET['action']) : 'frontpage';

    /* Generates Actions class filename (example: HomeActions) and path to
     * that class (example: home/HomeActions.php), checks if $file is a
     * valid file, and then, if so, requires that file.
     */
    $class = ucfirst($module) . 'Actions';
    $file  = $this->pageDir . '/' . $module . '/' . $class . '.php';

    try {

        //Checks for existance of file
        if (!is_file($file)) {
            throw new Exception('File not found!');
        }

        //Includes file
        require_once $file;

        /* Creates a new instance of the Actions class (example: $controller
         * = new HomeActions();), and passes the registry variable to the
         * ActionController class.
         */
        $controller = new $class();
        $controller->setRegistry($this->registry);

        //Trys the setModule method in the ActionController class
        $controller->setModule($module);

        /* The ActionController dispatchAction method checks if the method
         * exists, then runs the displayView function in the
         * ActionController class.
         */    
        $controller->dispatchAction($action);

    } catch(Exception $error) {

        /* An exception has occurred, and will be displayed if
         * $throwExceptions is set to true.
         */
        if($throwExceptions) {
            echo $error;
        }
    }
}
}

abstract class ActionController {

//Declaring variable(s)
protected $registry;
protected $module;
protected $registryItems = array();

//Class construct method
public function __construct(){}

public function setRegistry($registry) {

    //Sets the registry object
    $this->registry = $registry;

    /* Once the registry is loaded, the controller root directory path is
     * set from the registry.  This path is needed for the controller
     * classes to work properly.
     */
    $this->setPageDir();
}

//Sets the controller root directory from the value stored in the registry
public function setPageDir() {
    $this->pageDir = $this->registry->get('pageDir');
}

//Sets the module
public function setModule($module) {
    $this->module = $module;
}

//Gets the module
public function getModule() {
    return $this->module;
}

/* Checks for actionMethod in the Actions class (example: doFrontpage()
 * within home/HomeActions.php) with the method_exists function and, if
 * present, the actionMethod and displayView functions are executed.
 */  
public function dispatchAction($action) {
    $actionMethod = 'do' . ucfirst($action);
    if (!method_exists($this, $actionMethod)) {
        throw new Exception('Action not found!');
    }
    $this->$actionMethod();
    $this->displayView($action);
}

public function displayView($action) {
    if (!is_file($this->pageDir . '/' . $this->getModule() . '/' . $action . 'View.php')) {
        throw new Exception('View not found!');
    }

    //Sets $this->actionView to the path of the action View file
    $this->actionView = $this->pageDir . '/' . $this->getModule() . '/' . $action . 'View.php';

    //Sets path of the action View file into the registry
    $this->registry->set('actionView', $this->actionView);

    //Includes template file within which the action View file is included
    require_once $this->pageDir . '/default.tpl';
}
}

class Registry {

//Declaring variables
private $store;

//Class constructor
public function __construct() {}

//Sets registry variable
public function set($label, $object) {
    $this->store[$label] = $object;
}

//Gets registry variable    
public function get($label) {
    if(isset($this->store[$label])) {
        return $this->store[$label];
    } else {
        return false;          
    }
}

//Adds outside array of registry values to $this->store array
public function addRegistryArray($registryItems) {
    foreach ($registryItems as $key => $value) {
        $this->set($key, $value);
    }
}

//Returns registry array
public function getRegistryArray() {
    return $this->store;
}
}

4

3 回答 3

1

在没有详细查看您的代码的情况下:

尝试通过使用有意义的函数和变量名称来编写不言自明的代码。仅在代码的目的或功能不清楚的情况下使用注释。例如

//Declaring variable(s)
//Class construct method
//Checks for existance of file
//Includes file

是无用的注释,因为代码本身已经足够清晰了。

值得一读的书:干净的代码

于 2010-09-24T14:33:50.937 回答
1

我在过于本地化的密切投票和想要评论代码之间左右为难。另外,现在代码太多了,所以我只评论几件事:

1) 文档风格

为什么不使用已建立的文档格式,例如PHPDoc

2) 格式化

就我所见是一致的,但我建议使用广泛使用的编码约定,如PEAR或 ZF(基于 PEAR)而不是自己做(无论如何,你的接近 PEAR,所以你不妨完全采用它)。

3) 单例模式

为了让Singleton工作,它必须有一个私有的 __contruct 和 __clone 方法。但我建议根本不要使用它。许多人认为 Singleton 是一种反模式。有很多关于 SO 的问题都在讨论单例模式的缺点,所以看看周围。如果应该只有一个实例,那么就不要实例化第二个。如果你需要在其他类中访问 FrontController,注入它

4) 方法长度

我可能会尝试缩短dispatch方法。基本上,如果您描述一个方法的作用并且您必须使用and来这样做,那么该部分应该进入它自己的方法。尝试将方法制作成小的离散单元。这也将使单元测试更容易。

5) 关注点分离

我不确定为什么FrontController从 ActionController 扩展。没有其他类可以扩展它,但我想 FrontController 实例化的类也是 ActionController 的子类。但是 FrontController 虽然命名为控制器,但做的事情与PageControllers不同,所以我可能会将它们分开。

在旁注中,如果您对提高代码质量感兴趣,请查看http://phpqatools.org/上提供的幻灯片和工具

于 2010-09-24T14:44:55.560 回答
0

您的代码很像CakePhp。我建议检查一下,它使用 App::import() 和一个包装文件系统的 File 类来完成所有这些工作。它由社区在不同版本和操作系统上进行了测试,实际上可以节省您的时间。

于 2010-09-24T14:44:37.277 回答