读书人
栏目▼

两份PHP程序代码对比大家认为其编程

发布时间： 2013-01-28 11:49:56 作者： rapoo

两份PHP程序代码对比，大家认为其编程水平都怎么样？censor.class.php
功能：一个censor审核提交字符串的类（附加在DEDECMS里面）
两份不同的作品，同样的一种功能，大家评比一下~认为他们的水平如何？
如果你是主考官，你会如何选择？为什么？


涉及MySQL数据表：gk_info_censor


mysql> desc gk_info_censor;
+-------------+----------------------+------+-----+---------+----------------+
| Field       | Type                 | Null | Key | Default | Extra          |
+-------------+----------------------+------+-----+---------+----------------+
| id          | smallint(6) unsigned | NO   | PRI | NULL    | auto_increment |
| admin       | varchar(15)          | NO   |     |         |                |
| type        | smallint(6)          | NO   |     | 1       |                |
| find        | varchar(255)         | NO   | UNI |         |                |
| replacement | varchar(255)         | NO   |     |         |                |
| extra       | varchar(255)         | NO   |     |         |                |
| count       | int(11)              | NO   |     | 0       |                |
| updatetime  | int(11)              | NO   |     | NULL    |                |
| tids        | mediumtext           | NO   |     | NULL    |                |
| enable      | tinyint(1)           | NO   |     | -1      |                |
+-------------+----------------------+------+-----+---------+----------------+ 
 
10 rows in set (0.01 sec)



1.censor.class1.php

<?php

/* censorStatus: {banned} {censor} {replace} */

class Censor
{
  public $censorCount;
  public $censorData;
  public $censorLimit;

  function __construct()
  {
    $this->censorCount = NULL;
    $this->censorCount->replace = 0;
    $this->censorCount->censor = 0;
    $this->censorCount->Banned = 0;
    $this->censorLimit = 120;
    $this->censorData = $this->GetCensorData();
  }

  /*
  select * from
  */
  function GetCensorData()
  {
    static $censorData = NULL;
    if($censorData !== NULL) return $censorData;

    $censorData->banned = null;
    $censorData->censor = null;
    $censorData->replace= null;
    $censorData->repalce->from = $censorData->replace->to = null;

    global $dsql;
    $sql = "SELECT * FROM gk_info_censor WHERE replacement!='' AND `enable`=1 ";
    $dsql->Execute('s',$sql);
    while ($row = $dsql->GetArray('s'))
    {
      switch ($row['replacement'])
      {
        case '{banned}':  $censorData->banned[] = $row['find']; break;//banned
        case '{censor}':  $censorData->censor[] = $row['find']; break;//censor
        default:          $censorData->replace->from[] = $row['find'];//replace
                          $censorData->replace->to[] = $row['replacement'];
                          break;
      }
    }
    return $censorData;
  }

  function check( &$string )
  {
    if($this->banned( $string) < 1)
    {
      if($this->censor( $string ) < 1)
      {
        $this->replace( $string );
      }
    }
  }

  //替换级别 返回替换匹配个数
  function replace(&$subject)
  {
    if(empty($this->censorData->replace->from) || empty($this->censorData->replace->to)) return 0;

    $i = 0;
    while ($arrFROM = array_slice($this->censorData->replace->find,$i,$this->censorLimit)) 
 
    {
      $i += $this->censorLimit;
      if(empty($arrFrom)) continue;
      $arrTo = array_slice($this->censorData->replace->to,$i,$this->censorLimit);
      #$subject = preg_replace($arrFrom, $arrTo, $subject);
      array_map('str_replace',$arrFrom,$arrTo,$subject);
    }
  }

  //审核级别 返回审核匹配个数
  function censor(&$subject)
  {
    if(empty($this->censorData->censor)) return 0;

    $i = $count = 0;
    while ($arr = array_slice($this->censorData->censor,$i,$this->censorLimit))
    {
      preg_match_all('#'.split('|',$arr).'#', $subject, $matches);
      if($matches) $count += count($matches[0]);
      $i += $this->censorLimit;
    }
    $this->censorCount->banned += $count;
    return $count;
  }

  //彻底禁止 返回禁止匹配个数
  function banned(&$subject)
  {
    if(empty($this->censorData->banned)) return 0;

    $i = $count = 0;
    while($arr = array_slice($this->censorData->banned,$i,$this->censorLimit))
    {
      preg_match_all('#'.split('|',$arr).'#', $subject, $matches);
      if($matches) $count += count($matches[0]);
      $i+= $this->censorLimit;
    }
    $this->censorCount->banned += $count;
    return $count;
  }

  //审核级别 banned,censor,pass
  function censorLevel()
  {
    if($this->censorCount->banned > 0 ) return 'banned';
    if($this->censorCount->censor > 0 ) return 'censor';
    return 'pass';
  }
}



2.censor.class2.php
<?php

/* censorStatus: {banned} {censor} {replace} */

class Censor
{
  public $censorCount_banned;
  public $censorCount_censor;
  public $censorCount_replace;
  public $censorData;
  public $censorLimit;
  public $cachePath;

  function __construct()
  {
    $this->censorCount_replace = 0;
    $this->censorCount_censor = 0;
    $this->censorCount_banned = 0;
    $this->censorLimit = 120;
    $this->cachePath = DEDEDATA.'/cache_censor.php';
    $this->censorData = $this->GetCensorData();
  }


  function GetCensorData()
  {
    if(defined('CENSORDATA_LOADED'))
    { 
 
      if(empty($censorData)) ShowMsg("ERROR:CENSOR数据不存在！");
      return $censorData;
    } elseif( file_exists($this->cachePath) && filesize($this->cachePath) > 1024) {
      include $this->cachePath;
      if(empty($this->cachePath)) ShowMsg('Error: censor cache file size error!');
      return $censorData;
    }

    $censorData = array();
    $censorData['banned'] = array();
    $censorData['censor'] = array();
    $censorData['repalce']= array();

    global $dsql;
    $sql = "SELECT * FROM gk_info_censor WHERE replacement!='' AND `enable`=1 ";
    $dsql->Execute('s',$sql);
    while ($row = $dsql->GetArray('s'))
    {
      switch ($row['replacement'])
      {
        case '{banned}':  $censorData[banned][] = $row['find']; break;//banned
        case '{censor}':  $censorData[censor][] = $row['find']; break;//censor
        default:          $censorData[replace][$row['find']]=$row['replacement'];break;//replace
      }
    }

    #$cachePath = DEDEDATA.'/cache_censor.php';
    $cacheCode = "<"."?php\n";
    $cacheCode .= "\n#Make Time:".date("Y-m-d H:i:s");
    $cacheCode .= "\n#Make File:".__FILE__."(Line:".__LINE__.")";
    $cacheCode .= "\n#notice! do not modify me!\n\n";

    $cacheCode .= "\ndefine('CENSORDATA_LOADED',TRUE);";
    $cacheCode .= "\n\$censorData=array();";
    $cacheCode .= "\n\$censorData[banned] = null;";
    $cacheCode .= "\n\$censorData[censor] = null;";
    $cacheCode .= "\n\$censorData[replace]= null;";

    $cacheCode .= $this->makecode($censorData[banned],'censorData[banned]');
    $cacheCode .= $this->makecode($censorData[censor],'censorData[censor]');
    foreach ($censorData[replace] as $k => $v) {
      $cacheCode .= "\n\$censorData[replace]['".addslashes($k)."']= '".addslashes ($v)."';";
    }
    $cacheCode .= "\n\n?".">";

    $fp = @fopen($this->cachePath,"w") or die(" Tag Engine Create File FALSE! ");
    fwrite($fp,$cacheCode);
    fclose($fp);
    return $censorData;
  }

  function makecode($data,$varname)
  {
    $i=0;
    $codestring = '';
    while($arr = array_slice($data,$i,$this->censorLimit,true)) 
 
    {
      $codestring .= "\n\$".$varname."[] = '/". addcslashes( implode('|',$arr),"{}[]/.()'\"*" )."/i';";
      $i += $this->censorLimit;
    }
    return $codestring;
  }

  function check( &$string )
  {
    $this->banned($string);
    $this->censor($string);
    $this->replace($string);
    /*
    if($this->banned( $string) < 1)
    {
      if($this->censor( $string ) < 1)
      {
        $this->replace( $string );
      }
    }
    */
  }

  //替换级别 返回替换匹配个数
  function replace(&$subject)
  {
    if(empty($this->censorData[replace])) return 0;

    foreach($this->censorData[replace] as $from => $to)
    {
      $subject = str_replace($from,$to,$subject);
    }
  }

  //审核级别 返回审核匹配个数
  function censor(&$subject)
  {
    if(empty($this->censorData[censor])) return 0;

    $count = 0;
    foreach($this->censorData[censor] as $pattern)
    {
      preg_match_all($pattern, $subject, $matches);
      if($matches) $count += count($matches[0]);
    }
    $this->censorCount_censor += $count;
    return $count;
  }

  //彻底禁止 返回禁止匹配个数
  function banned(&$subject)
  {
    if(empty($this->censorData[banned])) return 0;

    $count = 0;
    foreach($this->censorData[banned] as $pattern)
    {
      preg_match_all($pattern, $subject, $matches);
      if($matches) $count += count( $matches[0] );
      //print_r($matches);
    }
    $this->censorCount_banned += $count;
    return $count;
  }

  //审核级别 banned,censor,pass
  function censorLevel()
  {
    if($this->censorCount_banned > 0 ) return 'banned';
    if($this->censorCount_censor > 0 ) return 'censor';
    return 'pass';
  }
}



[解决办法]
别的先不说
至少第二段代码 29 行处的
if(empty($censorData)) ShowMsg("ERROR:CENSOR数据不存在！");
return $censorData;


是错误的
[解决办法]
都很牛逼的样子
[解决办法]
都垃圾的很！
[解决办法]


路过~
[解决办法]
你就逗大家玩呢....

来人啊...脱出去那个....对了,你是男是女?



[解决办法]

//这一段建议包装一下。 如果你喜欢使用缓存策略，这块代码需要封装起来，便于重用！~
//读和取，都封起来~
    if(defined('CENSORDATA_LOADED') && !empty($GLOBALS['censorData']))
    {
      return $GLOBALS['censorData'];
    } elseif( file_exists($this->cachePath) && filesize($this->cachePath) > 1024) {
      include $this->cachePath;
      if(!empty($censorData)) return $censorData;
    }

    #$cachePath = DEDEDATA.'/cache_censor.php'; 
 
    $cacheCode = "<"."?php\n";
    $cacheCode .= "\n#Make Time:".date("Y-m-d H:i:s");
    $cacheCode .= "\n#Make File:".__FILE__."(Line:".__LINE__.")";
    $cacheCode .= "\n#notice! do not modify me!\n\n";
 
    $cacheCode .= "\ndefine('CENSORDATA_LOADED',TRUE);";
    $cacheCode .= "\n\$censorData=array();";
    $cacheCode .= "\n\$censorData['banned'] = null;";
    $cacheCode .= "\n\$censorData['censor'] = null;";
    $cacheCode .= "\n\$censorData['replace']= null;";
 
    $cacheCode .= $this->makecode($censorData['banned'],"censorData['banned']");
    $cacheCode .= $this->makecode($censorData['censor'],"censorData['censor']");

[解决办法]
明显就是一个人写的，命名风格都一样。

几点不足：

1 “banned”这种常量字符串，在class中我更喜欢用const来声明。如果它们是枚举值，应该定义一个数组来存放全部枚举值，类似创建了一个哈希表，这样更加严谨。还有那些写死的数字，都应该使用const来定义，调用时self::XXX;这样改的时候直接从类的头部改一个地方下面全改了。

2 虽然php没规定变量提前声明，但既然选择使用class，就应该规范一些。这样会造成一大堆警告。提前显示声明所有使用的变量，不要用隐式声明。

3 完全是php4的class用法，没有封装的概念！所有方法全都默认的public，就没有private或protected。如果这个类是个单一功能类。那么应该只有一个public方法负责把最后结果返回给外部，外部只需要调用这一个方法就够了。其他的全部private，由这个方法在类内部去调用那些private或protected方法，如果此类以后仍有扩展的空间，需要继承的话，应该用protected。

4 使用了全局变量，我认为在没有namespace的大多数情况下，都不应该使用全局变量，尤其是这种名称短的，什么$dsql,$GLOBALS['censorData']d的，尽量用参数传进来也不要用全局变量。

5 $this->censorData[replace]这种极为初级的问题也有。不加引号php解释器会尝试把replace当成常量去解释，这种写法比加上引号效率低很多。

6 $fp = @fopen($this->cachePath,"w") or die(" Tag Engine Create File FALSE! ");竟然还有这种代码。。。就不能事先判断下file_exists并且is_writable下。非要用那个@符号。

7 这代码好蛋疼。
function check( &$string ) {
    if($this->banned( $string) < 1)
    {
      if($this->censor( $string ) < 1)
      {
        $this->replace( $string );
      }
    }
  }

function banned(&$subject)
对于这种引用传递的。可以使用链式语法，这些方法都不需要再返回那个数量，而是在方法内部做数量检查，然后都返回$this。
然后这里就可以用链式语法了。

function banned(&$subject){
    ...
    return $this;
}

function check( &$string ) {
    $this->banned( $string)
            ->censor( $string )
            ->replace( $string )
            ->
}



总结，对oop的理解很肤浅。除非用的是php4，否则这样使用class还不如直接写一堆函数。明显是为了用class而用的class，根本没有理解oop的精髓。有些方法可以拆开几个方法。虽然有些嗦，但那样才符合抽象，松耦合的概念。包括你改过之后的代码，仅仅是实现了功能而已，从oop的角度讲，还欠缺很多。
[解决办法]
各有千秋,不过，总体看第一个比较好
读书人网 >PHP

热点推荐

触屏版 | 电脑版

读书人网 m.reader8.net