该库位于此处
如果您正在寻找从哪里开始,请检查
App.php
文件夹中的AppController.php
和classes
我真的很想听听您对以下问题的批评:
代码质量
代码清晰度
如何改进
需要澄清扩展等的其他内容
我也听说我的静态方法(例如
get
中的set
和App
方法)应替换为更具动态性的东西,我该怎么做那是什么?我对做错的事情比对的更感兴趣。
欢迎对库的实际用途发表任何看法。
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;
}
}
?>
#1 楼
我已经花了大约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;
两点:
要么对每个私有/受保护的变量和函数都这样做,要么根本不做
根本不做
第二点有些道理:
每一个像样的IDE都可以基于变量进行排序他们的可见度/范围。 Eclipse更进一步,在“大纲”视图中将绿色用于公共,将红色用于私有/公共,因此所有内容都易于识别。
在PHP函数中加下划线的前缀可能会误认为魔术方法(以两个下划线为前缀)。
请注意,尽管几乎所有的php IDE都将对待:
function foo()
as:
public function foo()
您可以通过添加public关键字来帮助IDE。这也是一个易读性点,并且是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注释,每个功能都需要一个注释。但是请注意,我一直被指控过分评论:)
PHP4样式类成员
您正在使用一些PHP4样式类成员:
var $name = __CLASS__;
var $helpers = array();
var $validate = array();
var $posts = array();
您应该在各处使用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 whaty),并且显然仍在进行中(尤其是AppController.php)。您需要尝试并至少以更一致的方式重写它。
PS。正如leo建议的那样,您应该从每个类脚本的末尾删除
?>
。#2 楼
您的代码看起来非常不错,并且也具有不错的PHP-Doc,但是我认为方法内的注释有点令人困惑,因为它们始终具有不同的书写风格(有时大写,有时小写,有时有时只是简单的单词等) 。),因此您应该清理它们并设置约定,以便在代码中记录这些点。此外,您还应该对
load_view
中的foreach循环有点谨慎,因为在那里设置了$$key = $variable;
。如果您知道要为控制器设置什么,那还不错,但是如果尝试设置$this
会怎样呢?我没有尝试过,但是它实际上会产生一个错误,因此我建议您只创建一个保留变量数组,在加载视图之前将对其进行检查。在那里,您还应该添加$cwd
和$template_file
,也许还有更多这样的vars。我实际上在您的代码中看到的是一些封闭的PHP标记,这只是一些编码约定在文件末尾(
?>
)。当然,我看到许多开发人员现在正在使用该功能,但是我建议您不要使用它,您可以添加诸如/* End of File */
之类的注释。问题是,如果在关闭PHP标记后有一个空格,然后尝试修改标头,它将无法正常工作,只会出现错误。并替换您的
get
可以使用PHP魔术方法完成set
和__get($var)
方法。应使用__set($var)
和$this->newTemplateVar = "Hello World!";
,而不是像普通对象属性一样访问它们。例如。 q4312079q。希望我的帖子对您有所帮助;如果没有,就说出来。
#3 楼
我同意扬尼斯。但是最后一件事。构造函数只是应该为属性分配值。您的构造函数似乎要做的还不止这些。将其切换为接受数据库连接。请记住,静电是有害的,因为它们会使测试变得困难。
$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);
评论
\ $ \ begingroup \ $
哇。太彻底了!看看这个相关的SO问题:stackoverflow.com/questions/7676515/…
\ $ \ endgroup \ $
– Naftali又名Neal
2011年11月9日14:01
\ $ \ begingroup \ $
我已经更新了一些图书馆^ _ ^
\ $ \ endgroup \ $
– Naftali又名Neal
2011年12月8日在21:57
\ $ \ begingroup \ $
看到我更新的代码,您认为我应该提出一个新的问题吗?
\ $ \ endgroup \ $
– Naftali又名Neal
2012年1月3日在22:53
\ $ \ begingroup \ $
我只是希望它不会作为重复项而关闭(我正在解决即将在我的更新中出现的下划线问题,大部分情况下切换到所有camelCase)
\ $ \ endgroup \ $
– Naftali又名Neal
2012年1月5日15:43
\ $ \ begingroup \ $
@Neal我不认为这是重复的,您不只是复制粘贴相同的代码,而且还做了很多修改。关于在Code Review中构成欺骗的讨论仍在进行中,所以我不能确定地说...但是无论如何都可以这样做。
\ $ \ endgroup \ $
– yannis
2012年1月5日15:46