Небольшой PHP в шаблон просмотра/контроллер


Я написал небольшой шаблон просмотр/контроллер ЭСК библиотека, что я хотел бы какие-то критические статьи на.

Библиотека расположена здесь

Если вы ищете, где начать, проверить App.php и AppController.php в классы папку

Я бы очень хотела услышать вашу критику на:

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

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

Какие мнения по поводу реальной полезности библиотеки приветствуются.

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;
    }
}

?>


1651
28
задан 28 сентября 2011 в 04:09 Источник Поделиться
Комментарии
3 ответа

Я потратил около 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
}
}

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

Имена функций нестыковки

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

protected $_mysql;

Два пункта:


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

  2. Не делай этого вообще

Какое-то оправдание по второму пункту:


  • Каждый приличный язь там позволяет для сортировки переменных и функций на основе их visibillity / объем. Затмение идет еще дальше и использует зеленый, и красный для частного / общественного В схема, так все легко идентифицировать.

  • В PHP функции, которые начинаются с символа подчеркивания, могут быть приняты за "магические" методы (которые, начинающиеся с двух подчеркиваний).

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

function foo() 

как:

public function foo()

вы могли бы помочь в IDE, добавив ключевое слово Public. Это тоже readibillity точки и РНР4 стиль (функция Foo()) против РНР5 стиль (общественная функция 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 массив не инициализирован) если вызывать из любого места, где если же нет() не был вызван. Вы можете сделать что-то вроде:

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

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

Комментарии

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

Для PHP4 стиль члены класса

Вы используете PHP4 с типа членов класса:

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

Вы должны использовать РНР5 контроля доступа (публичная / частная / защищенные члены) повсюду.

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

Это:

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;
}
}

остальное не нужно (как , если блок возвращает, что означает, что выполнение функции останавливается там):

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 параметр, который содержит объект. Переименовать второй в $контроллер.

В заключение

Ваш код имеет много мелких нестыковок, несколько вопросов readabillity, несколько крупных проблем (особенно в формате HTML штуку) и это явно еще не доработана (особенно AppController.php). Вам нужно попробовать и хотя бы переписать его в более последовательный вопрос.

ПС. Вы должны удалить ?> в конце каждой класса сценарий, как Лео говорит.

18
ответ дан 9 ноября 2011 в 04:11 Источник Поделиться

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

Также вы должны быть немного внимательней с foreach в load_view, потому что там вы установите $$ключ = $переменная;. Если вы знаете, что вы устанавливаете в контроллер, этом не ничего плохого, но что, если вы попытаетесь установить это? Я не пробовал это, но на самом деле он будет выдавать ошибку, так что я бы порекомендовал вам просто создать массив зарезервированные переменные, которые будут проверены перед загрузкой посмотреть. Там вы также должны добавить $чвд и $template_file, возможно, есть еще ВАР, как эти.

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

А замену вам и набор методов может быть сделано с помощью PHP магические методы.. Вы должны использовать __вам (из$VAR) и __набор($VAR), на чем вы могли получить к ним доступ, как обычные свойства объекта. Е. Г. $этом->newTemplateVar = "Привет Мир!";.

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

4
ответ дан 10 октября 2011 в 10:10 Источник Поделиться

Я согласен с Янисом. Но 1 Последнее дело.

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

$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);

0
ответ дан 24 февраля 2014 в 04:02 Источник Поделиться