Малый шаблон просмотра /контроллера PHP

Я написал небольшую библиотеку шаблонов Viewer /Controller esque, на которые я бы хотел остановиться.

Библиотека находится здесь

Если вы ищете, с чего начать, ознакомьтесь с App.php и AppController.php в папке classes

Мне бы очень хотелось услышать ваши критические замечания:

  • Качество кода
  • Ясность кода
  • Как улучшить
  • Все, что нуждается в расширении разъяснений и т. д.
  • Я также слышал, что мои статические методы (например, методы get и set в App)) должны быть заменены чем-то большим dynamic , как бы я это сделал?

Меня больше интересует то, что я делаю неправильно, чем право.

Любые мнения о фактической пользе библиотеки приветствуются.

App.php :

<?php
/**
 * Description of App
 *
 * @author nlubin
 */
class App {
    /**
     * Holds all of the app variables
     * @var array
     */
    private static $app_vars = array();
    /**
     * Will be an App object
     * @var App
     */
    private static $app = null;

    /**
     * Get a single app_vars variable
     * @param string $v
     * @return mixed 
     */
    public static function get($v){
        return isset(self::$app_vars[$v])?self::$app_vars[$v]:false;
    }

    /**
     * Get all app_vars variables
     * @return array app_vars
     */
    public static function getAll(){
        return self::$app_vars;
    }

    /**
     * Set an app_vars variable
     * 
     * @param string $v
     * @param mixed $va
     * @return mixed
     */
    public static function set($v, $va){
        if(self::$app == null){ //create App on first set. if not, the app does not exist
            self::$app = new self();
        }
        return self::$app_vars[$v] = $va;
    }

    /**
     * Clean up the app_vars variable
     */
    public static function clean(){
        self::$app_vars = array();
    }

    public function __construct() {
        $this->_connection = Database::getConnection();
    }

    private function render_template(){
        $rPath = $this->read_path();
        foreach($rPath as $key=>$value){
            $$key = $value;
        }
        unset($rPath);

        ob_start();

        App::set('page_title',App::get('DEFAULT_TITLE'));
        App::set('template',App::get('DEFAULT_TEMPLATE'));
        App::set('page',$page);

        //LOGIN
        if(!isset($_SESSION['LOGIN']) || $_SESSION['LOGIN'] == false){
            Login::check_login();
        }
        else {
            $modFolders = array('images', 'js', 'css');

            //load controller
            if(strlen($controller) == 0) $controller = App::get('DEFAULT_CONTROLLER');

            if(count(array_intersect($path_info, $modFolders)) == 0){ //load it only if it is not in one of those folders
                $controllerName = "{$controller}Controller";
                $app_controller = $this->create_controller($controllerName, $args); 
            }
            else {  //fake mod-rewrite
                $this->rewrite($path_info);
            }
        }

        $main = ob_get_clean();
        App::set('main', $main);
        //LOAD VIEW
        ob_start();
        $this->load_view($app_controller, 0);
        //END LOAD VIEW

        //LOAD TEMPLATE
        $main = ob_get_clean();
        App::set('main', $main);
        $this->load_template($app_controller, $app_controller->get('jQuery'));
        //END LOAD TEMPLATE
    }


    private function read_path(){
        $path = isset($_SERVER["PATH_INFO"])?$_SERVER["PATH_INFO"]:'/'.App::get('DEFAULT_CONTROLLER');
        $path_info = explode("/",$path);
        $page = (isset($path_info[2]) && strlen($path_info[2]) > 0)?$path_info[2]:'index';
        list($page, $temp) = explode('.', $page) + array('index', null);
        $args = array_slice($path_info, 3);
        $controller = isset($path_info[1])?$path_info[1]:App::get('DEFAULT_CONTROLLER');
        return array(
            'path_info'=>$path_info,
            'page'=>$page,
            'args'=>$args,
            'controller'=>$controller
        );
    }

    private function create_controller($controllerName, $args = array()){
        if (class_exists($controllerName)) {  
            $app_controller  = new $controllerName(); 
        } else {
            //show nothing 
            header("HTTP/1.1 404 Not Found");
            exit;
        }
        echo $app_controller->display_page($args);
        return $app_controller;
    }

    private function load_template($controllerName, $jQuery = null){

        $page_title = $controllerName->get('title')?$controllerName->get('title'):App::get('DEFAULT_TITLE');
        //display output
        $cwd = dirname(__FILE__);
        $template_file = $cwd.'/../view/'.App::get('template').'.stp';
        if(is_file($template_file)){
            include $template_file;
        }
        else {
            include $cwd.'/../view/missingfile.stp'; //no such file error
        }
    }

    private function load_view($controllerName, $saveIndex){

        //Bring the variables to the global scope
        $vars = $controllerName->getAll();
        foreach($vars as $key=>$variable){
            $$key = $variable;
        }
        $cwd = dirname(__FILE__);
        if(App::get('view')){
            $template_file = $cwd.'/../view/'.App::get('view').'/'.App::get('method').'.stp';
            if(is_file($template_file)){
                include $template_file;
            }
            else {
                include $cwd.'/../view/missingview.stp'; //no such view error
            }
        }
        else {
            App::set('template', 'blank');
            include $cwd.'/../view/missingfunction.stp'; //no such function error
        }
    }


    private function rewrite($path_info){
        $rewrite = $path_info[count($path_info) - 2];
        $file_name = $path_info[count($path_info) - 1];

        $file = WEBROOT.$rewrite."/".$file_name;
//                echo $file; 
        header('Location: '.$file);
        exit;
    }

    public function __destruct() {
        $this->render_template();
    }
}

?>

AppController.php

<?php
/**
 * Description of AppController
 *
 * @author nlubin
 */
class AppController {

    /**
     *
     * @var mySQL
     */
    protected $_mysql;
    protected $_page_on,
            $_allowed_pages = array(),
            $_not_allowed_pages = array(
                '__construct', 'get', 'set', 
                'getAll', 'display_page', 'error_page',
                'include_jQuery', 'include_js', '_setHelpers',
                '_validate_posts', '_doValidate', '_make_error'
            );
    protected $app_vars = array();
    var $name = __CLASS__;
    var $helpers = array();
    var $validate = array();
    var $posts = array();
    protected $validator;

    public function __construct()   {
        $this->_mysql = Database::getConnection();
        $this->_page_on = App::get('page');
        App::set('view', strtolower($this->name));
        $this->_allowed_pages = get_class_methods($this);
        $this->set('jQuery', $this->include_jQuery());
        $this->setHelpers();
        $this->validator = new FormValidator();
        $this->_validate_posts();
        $this->posts = (object) $this->posts;
        if(!isset($_SESSION[App::get('APP_NAME')][strtolower($this->name)])){
            $_SESSION[App::get('APP_NAME')][strtolower($this->name)] = array();
        }
        return;
    }

    public function init(){

    }

    public function get($v){
        return isset($this->app_vars[$v])?$this->app_vars[$v]:false;
    }

    protected function set($v, $va){
        return $this->app_vars[$v] = $va;
    }

    public function getAll(){
        return $this->app_vars;
    }
    /**
     * Show the current page in the browser
     * @return string 
     */
    public function display_page($args)  {
        App::set('method', $this->_page_on);
        $private_fn = (strpos($this->_page_on, '__') === 0);
        if(in_array($this->_page_on, $this->_allowed_pages) 
                && !in_array($this->_page_on, $this->_not_allowed_pages)
                        && !$private_fn)    {  
            $home = $this->include_jQuery();
            call_user_func_array(array($this, $this->_page_on), $args);
        }
        else    {
            if(App::get('view') == strtolower(__CLASS__) || $private_fn ||
                    in_array($this->_page_on, $this->_not_allowed_pages)){
                header("HTTP/1.1 404 Not Found");
            }
            else {
                App::set('method', '../missingfunction'); //don't even allow trying the page
                return($this->error_page(App::get('view')."/{$this->_page_on} does not exist."));
            }
            exit;
        }
    }

    /**
     *
     * @return string 
     */
    function index()    {}

    /**
     *
     * @param string $msg
     * @return string 
     */
    protected function error_page($msg = null)    {
        $err = '<span class="error">%s</span>';
        return sprintf($err, $msg);
    }

    /**
     *
     * @return string 
     */
    protected function include_jQuery(){
        $ret = '<script type="text/javascript" src="js/jquery-1.6.2.min.js"></script>'.PHP_EOL;
        $ret .= '        <script type="text/javascript" src="js/jquery-ui-1.8.9.custom.min.js"></script>'.PHP_EOL;
        return $ret;
    }

    /**
     *
     * @param string $src
     * @return string 
     */
    protected function include_js($src){
        $script = '<script type="text/javascript" src="js/%s.js"></script>'.PHP_EOL;
        return sprintf($script, $src);
    }

    protected function setHelpers(){
        $helpers = array();
        foreach($this->helpers as $helper){
            $help = "{$helper}Helper";
            $this->$helper = new $help();
            $helpers[$helper] = $this->$helper;
        }
        self::set('helpers', (object) $helpers);
    }

    protected function logout(){
        session_destroy();
        header('Location: '.WEBROOT.'index.php');
        exit;
    }

    protected function _validate_posts(){
        foreach($this->validate as $field => $rules){
            foreach($rules as $validate=>$message){
                $this->validator->addValidation($field, $validate, $message);
            }
        }
        $this->_doValidate();
    }

    protected function _doValidate(){
        if(!(!isset($_POST) || count($_POST) == 0)){
            //some form was submitted
            if(!$this->validator->ValidateForm()){
                $error = '';
                $error_hash = $this->validator->GetErrors();
                foreach($error_hash as $inpname => $inp_err)
                {
                  $error .= "$inp_err<br/>\n";
                }
                $this->_make_error($error);                
            }

            foreach($_POST as $key=>$post){
                $this->posts[$key] = $post;
            }
        }
    }

    function __get($var_name){
//        echo $var_name."<br>";
        if(isset($this->posts->$var_name)){
            return $this->posts->$var_name;
        }
        else{
            ?><div class="errors"><?php
               echo "$var_name is not set<br/>\n";
            ?></div><?php
            exit;
        }
    }

    function __call($name, $arguments){
        if($name == 'mysql'){
            return (strlen($this->$arguments[0])==0?"NULL":"'{$this->$arguments[0]}'");
        }
    }

    function _make_error($str){
        ?><div class="errors"><?php
        echo $str;
        ?></div><?php
        exit;
    }
}

?>
28 голосов | спросил Neal 28 thEurope/Moscowp30Europe/Moscow09bEurope/MoscowWed, 28 Sep 2011 20:03:50 +0400 2011, 20:03:50

3 ответа


18

Я потратил около 20 минут на чтение вашего кода, и я определил несколько проблем.

Относительные пути

private function load_template($controllerName, $jQuery = null){
    $page_title = $controllerName->get('title') ? $controllerName->get('title') : App::get('DEFAULT_TITLE');

    //display output
    $cwd = dirname(__FILE__);
    $template_file = $cwd.'/../view/'.App::get('template').'.stp';
    if(is_file($template_file)){
        include $template_file;
    }
    else {
        include $cwd.'/../view/missingfile.stp'; //no such file error
    }
}   

Что произойдет, если позднее вы решите изменить структуру своего каталога? Вместо использования dirname(__FILE__) вы можете просто передать этот базовый каталог как параметр функции.

Несоответствия имен функций

Вы префикс некоторые частные /защищенные переменные и функции с подчеркиванием:

protected $_mysql;

Две точки:

  1. Либо делать это для каждой частной /защищенной переменной и функции или вообще не делать этого
  2. Не делайте этого вообще

Некоторые обоснования второй точки:

  • Каждая достойная IDE позволяет сортировать переменные и функции на основе их видимости /объема. Eclipse продвигается дальше и использует зеленый для публики, а красный для частного /публичного в представлении Outline, поэтому все легко идентифицируется.
  • В функциях PHP, которые имеют префикс с подчеркиванием, могут ошибочно приниматься за магические методы (которые префиксны двумя символами подчеркивания).

Обратите внимание, что, хотя почти все PHP-IDE будут обрабатывать:

function foo() 

как:

public function foo()

вы можете помочь IDE, добавив ключевое слово public. Это также точка читаемости и стиль PHP4 (function foo()) и стиль PHP5 (public function foo()).

Встроенный HTML

protected function error_page($msg = null)    {
    $err = '<span class="error">%s</span>';
    return sprintf($err, $msg);
}

protected function include_jQuery(){
    $ret = '<script type="text/javascript" src="js/jquery-1.6.2.min.js"></script>'.PHP_EOL;
    $ret .= '        <script type="text/javascript" src="js/jquery-ui-1.8.9.custom.min.js"></script>'.PHP_EOL;
    return $ret;
}

function __get($var_name){
    // echo $var_name."<br>";
    if(isset($this->posts->$var_name)){
        return $this->posts->$var_name;
    }
    else{
        ?><div class="errors"><?php
           echo "$var_name is not set<br/>\n";
        ?></div><?php
        exit;
    }
}   

Не делайте этого, вы должны отдельная презентация из логики .

Сессия

В AppController.php вы используете сеансы:

if(!isset($_SESSION[App::get('APP_NAME')][strtolower($this->name)])){
    $_SESSION[App::get('APP_NAME')][strtolower($this->name)] = array();
}

Но у вас нет session_start() в начале скрипта. Конечно, вы, вероятно, начинаете сеанс в сценарии вызывающего абонента, но это означает, что ваш класс будет вызывать уведомления ($_SESSION массив не инициализирован), если вызывается где угодно, где session_start() не был вызван. Вы можете сделать что-то вроде:

if( !isset($_SESSION) ) session_start();

вверху каждого скрипта, который использует сеансы.

Комментарии

У вас есть комментарии phpDoc, вам нужна одна функция every . Но, пожалуйста, обратите внимание, что я был обвиняемым в преувеличении: )

Члены класса стиля PHP4

Вы используете некоторыеЧлены класса стиля PHP4:

var $name = __CLASS__;
var $helpers = array();
var $validate = array();
var $posts = array();

Вы должны всегда использовать элементы управления доступом (public /private /protected) PHP5.

Тройная читаемость оператора

Это:

return isset(self::$app_vars[$v])?self::$app_vars[$v]:false;

не читается. Рассмотрим что-то вроде:

return
    isset(self::$app_vars[$v])
    ? self::$app_vars[$v]
    : false;

или по крайней мере что-то вроде этого:

return isset(self::$app_vars[$v]) ? self::$app_vars[$v] : false;

Аналогично, добавьте пробелы до и после точек конкатенации:

.. $cwd.'/../view/'.App::get('template').'.stp'; // somewhat unreadable $cwd . '/../view/'.App::get('template') . '.stp'; // friendlier to the eyes

Незначительный материал

private function load_template($controllerName, $jQuery = null) {}

Вы не используете переменную $jQuery в этой функции. Ваш код завершен? Если нет, вы не должны размещать его для просмотра.

function __get($var_name){
    // echo $var_name."<br>";
    if(isset($this->posts->$var_name)){
        return $this->posts->$var_name;
    }
    else{
        ?><div class="errors"><?php
           echo "$var_name is not set<br/>\n";
        ?></div><?php
        exit;
    }
}

else не требуется (как if block, что означает, что выполнение функции останавливается):

function __get($var_name){
    // echo $var_name."<br>";
    if(isset($this->posts->$var_name)){
        return $this->posts->$var_name;
    }

    ?><div class="errors"><?php
       echo "$var_name is not set<br/>\n";
    ?></div><?php
    exit;        
}

Здесь:

private function create_controller($controllerName, $args = array()){
    if (class_exists($controllerName)) { 

Вы обрабатываете $controllerName как параметр, который содержит имя класса, но в самой следующей функции:

private function load_template($controllerName, $jQuery = null){
    $page_title = $controllerName->get('title')?$controllerName->get('title'):App::get('DEFAULT_TITLE');

у вас также есть параметр $controllerName, который содержит объект. Переименуйте второй код в $controller.

В заключение

В вашем коде есть много незначительных несоответствий, несколько проблем с читабельностью, несколько серьезных проблем (особенно html thingy), и это, очевидно, продолжает работу (особенно AppController.php). Вам нужно попробовать и хотя бы переписать его в более согласованном виде.

PS. Вы должны удалить ?> с конца каждого скрипта класса, поскольку leo предлагает .

ответил yannis 9 32011vEurope/Moscow11bEurope/MoscowWed, 09 Nov 2011 08:08:53 +0400 2011, 08:08:53
4

Ваш код выглядит здорово, и он также имеет хороший PHP-Doc, но я думаю, что комментарии внутри методов немного запутывают, потому что у них всегда есть другой стиль письма (иногда прописные, иногда строчные, иногда предложения иногда просто простые слова и т. д.), поэтому вы должны очистить их и установить соглашение о том, как вы документируете эти точки в коде.

Также вы должны быть немного осторожны с циклом foreach в load_view, потому что там вы устанавливаете $$key = $variable;. Если вы знаете, что вы настраиваете на контроллер, это не плохо, но что, если вы попытаетесь установить $this? Я не пробовал, но на самом деле возникла бы ошибка, поэтому я бы рекомендовал вам просто создать массив зарезервированных переменных, который будет проверяться перед загрузкой представления. Там вы также должны добавить $cwd и $template_file, возможно, есть больше таких варов.

Кроме того, что-то, что я действительно вижу в вашем коде, что всего лишь условное кодирование, является закрывающим тегом PHP в конце файла (?>). Конечно, я вижу, что многие разработчики используют это прямо сейчас, но я рекомендую вам уйти от этого, вместо этого вы можете просто добавить комментарий вроде /* End of File */. Проблема в том, что если у вас есть пробел после закрытия PHP-тега, и вы попытаетесь изменить заголовок позже, это не сработает, вы просто получите ошибки.

И замена ваших методов get и set может быть выполнена с помощью Магические методы PHP. . Вы должны использовать __get($var) и __set($var), чем вы могли бы получить к ним доступ, как обычные свойства объекта. Например. $this->newTemplateVar = "Hello World!";.

Я надеюсь, что мой пост поможет вам; если нет, просто скажите это.

ответил evotopid 10 +04002011-10-10T14:02:47+04:00312011bEurope/MoscowMon, 10 Oct 2011 14:02:47 +0400 2011, 14:02:47
0

Я согласен с яннисом. Но последнее.

Конструкторы просто должны назначать значения свойствам. Ваш конструктор, кажется, делает намного больше. Переключите его, чтобы принять соединение с базой данных. Помните, что статика - это зло, потому что они затрудняют тестирование.

$Settings   = new FileSettings(new File('Config.php'));
$MFactory   = new ModelFactory;
$TFactory   = new TemplateFactory($Settings['UI']['Template']);

$Database   = $MFactory->createDB($Settings['DB']['Driver'], ....);
$Request    = $MFactory->createRequest($_SERVER['PHP_SELF'], $_GET, $_POST);
$View       = $TFactory->createView($Request->getView());

$Controller = new Controller($Database, $TFactory, $MFactory);
$Action     = $Request->getAction()?: 'index';

if (is_callable(array($Controller, $Action))) {
    call_user_function(array($Controller, $Action));
}

else $Controller->error($Request);
ответил mAsT3RpEE 24 FebruaryEurope/MoscowbMon, 24 Feb 2014 20:58:52 +0400000000pmMon, 24 Feb 2014 20:58:52 +040014 2014, 20:58:52

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132