一次代码简化记录
开发功能时候,读到一个代码文件,发现里面代码重复率极高,便对其进行了一次优化,本文简述该优化过程,没有什么高深的知识,只是一些开发经验之谈。
没有对该文件进行简化之前,看了下那个文件的修改记录,是经过几次修改而“进化”过来的。
我们先从该文件的最初功能说起。
某一个统计主题属性类,统计信息和数据库关联着。最初只有一个功能,就是统计该主题的点赞数量。最开始,那个统计类只有增加点赞数量的接口,所以,最开始那个文件的功能大概就是这样的(代码简化隐藏了一些数据):
class TopicStatsModel extends BaseModel
{
public $topic_id;
public $likes_count;
public function incLikesCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->likes_count += $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update likes_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->likes_count = $cnt;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert likes_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
}
开始这个功能,这个文件还是正常的,后来,业务需求增加,需要点赞能够取消,这里就需要支持统计记录点赞的减少接口了,该类增加了一个减少点赞的接口,如下:
public function decLikesCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->likes_count -= $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update likes_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->likes_count = 0;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert likes_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
到这里该统计类就两个接口了,一个是增加赞,一个是减少赞。查看这两个接口,发下这两个接口大部分代码是相同的,现在先不说优化,继续看。
后来,业务继续增加,需要记录主题的评论数。这个时候,该类文件继续被修改,有增加了两个接口:
public function incCommentsCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->comments_count += $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update comments_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->comments_count = $cnt;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert comments_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function decCommentsCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->comments_count -= $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update comments_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->comments_count = 0;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert comments_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
需求继续增加,后来需要统计订阅数、赞赏数、阅读数……通过查看代码变更记录,看到了代码慢慢的被增加,增加了incSubscribeCount\decSubscribeCount、incRewardCount\decRewardCount、incReadCount\decReadCount 。
这几个函数对,和上面的函数对样子是95% 是一样的,只是操作不同的变量。这个时候这个文件的样子,代码量的重复率是多少,可以想象出来,看下面。
简化前的完整代码:
class TopicStatsModel extends BaseModel
{
public $topic_id;
public $comments_count;
public $rewards_count;
public $likes_count;
public $subscriptions_count;
public $read_count;
public function incRewardCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->rewards_count += $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update rewards_count failed, topic_id=$topic_id");
return false;
}
} else {
$this->rewards_count = $cnt;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert rewards_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function decRewardCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->rewards_count -= $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update rewards_count failed, topic_id=$topic_id");
return false;
}
} else {
$this->rewards_count = 0;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert rewards_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function incSubscribeCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->subscriptions_count += $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update subscriptions_count failed, topic_id=$topic_id");
return false;
}
} else {
$this->subscriptions_count = $cnt;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert subscriptions_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function decSubscribeCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->subscriptions_count -= $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update subscriptions_count failed, topic_id=$topic_id");
return false;
}
} else {
$this->subscriptions_count = 0;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert subscriptions_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function incLikesCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->likes_count += $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update likes_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->likes_count = $cnt;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert likes_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function decLikesCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->likes_count -= $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update likes_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->likes_count = 0;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert likes_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function incCommentsCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->comments_count += $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update comments_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->comments_count = $cnt;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert comments_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function decCommentsCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->comments_count -= $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update comments_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->comments_count = 0;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert comments_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function incReadCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->read_count += $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update read_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->read_count = $cnt;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert read_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
public function decReadCount($topic_id, $cnt = 1)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
$stats->read_count -= $cnt;
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update read_count failed, topic_id=$topic_id");
return false;
}
}
else {
$this->read_count = 0;
$this->topic_id = $topic_id;
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert read_count failed, topic_id=$topic_id");
return false;
}
$stats = $this;
}
return true;
}
}
到这里,可以看到该代码的几个接近的功能,代码有多么的重复了,也可以看出,代码维护者对于新功能的增加,只是增加一个变量,之后把原来的接口拷贝两个出来,修改一下命名,继续使用。这个代码维护方式,做法不能说有错误,但是,就是少了求精求优的思想了。
面对重复的代码,如果某处逻辑出错,对于拷贝出来的也肯定跟着出错了。
该代码文件的历史,就说到这,下面开始着手优化。
————————————-画一条分割线————————————-
* 优化1
先看看每个inc 和dec 函数对,稍微思考一下,会知道这个dec 函数是完全没有必要写的,为什么呢?
两函数几乎完全一样,不同之处就是在$stats->XXX_count += $cnt;和$stats->comments_count -= $cnt;
看出什么了没有?把这个dec 函数删掉也没有关系,需要调用dec 功能的函数时候,直接调用inc 的,把$cnt 除以负1 ,再传入去即可。
也就是说调用:decLikesCount($topic_count,1)和调用incLikesCount($topic_count,-1)是等价的。恩,inc 也需要稍微做下修改即可,放在更新后数据库的count 变成负数。
* 优化2
之前的文章《聊聊如何编写可读性强、可维护性易的代码》有提过,大面积相同的代码段,应该抽成函数作为公共代码。看下,点赞数增减、阅读数增减、收藏数增减等,这些再逻辑几乎是完全一样的,不同的只是对不同的变量进行修改。对此,很容易提取公共代码出来,从而进行代码的大大简化。
我们可以增加一个increaseStats($topic_id, $cnt, $field) 私有函数,该函数只有给本类内部调用。对外的借口通过调用该函数传入$field 来控制对不同变量的操作,即可达到了代码量的大大简化了。
$field也是定义类内部私有的标记,对外不暴露即可,他的取值就是如下类内的那几个const 字段:
简化后的完整代码:
class TopicStatsModel extends BaseModel
{
public $topic_id;
public $comments_count;
public $rewards_count;
public $subscriptions_count;
public $likes_count;
public $read_count;
const FIELD_COMMENTS_COUNT = "COMMENTS_COUNT";
const FIELD_REWARDS_COUNT = "REWARDS_COUNT";
const FIELD_SUBCRIPTIONS_COUNT = "SUBCRIPTIONS_COUNT";
const FIELD_LIKES_COUNT = "LIKES_COUNT";
const FIELD_READ_COUNT = "READ_COUNT";
private function increaseStats($topic_id, $cnt, $field)
{
if (!is_numeric($topic_id)) {
$logger->notice(__METHOD__ . " invalid topic_id=$topic_id");
return false;
}
$stats = $this->findFirstByTopicId($topic_id);
if ($stats) {
if (MiGroupTopicStats::FIELD_COMMENTS_COUNT == $field) {
$stats->$comments_count += $cnt;
} elseif (MiGroupTopicStats::FIELD_REWARDS_COUNT == $field) {
$stats->$rewards_count += $cnt;
} elseif (MiGroupTopicStats::FIELD_LIKES_COUNT == $field) {
$stats->$likes_count += $cnt;
} elseif (MiGroupTopicStats::FIELD_READ_COUNT == $field) {
$stats->$read_count += $cnt;
} elseif (MiGroupTopicStats::FIELD_SUBCRIPTIONS_COUNT == $field) {
$stats->$subscriptions_count += $cnt;
}
if (!$stats->save()) {
$logger->notice(__METHOD__ . " update $field failed, topic_id=$topic_id");
return false;
}
} else {
if ($cnt < 0) {
return true;
}
$this->topic_id = $topic_id;
if (MiGroupTopicStats::FIELD_COMMENTS_COUNT == $field) {
$this->$comments_count = $cnt;
} elseif (MiGroupTopicStats::FIELD_REWARDS_COUNT == $field) {
$this->$rewards_count = $cnt;
} elseif (MiGroupTopicStats::FIELD_LIKES_COUNT == $field) {
$this->$likes_count = $cnt;
} elseif (MiGroupTopicStats::FIELD_READ_COUNT == $field) {
$this->$read_count = $cnt;
} elseif (MiGroupTopicStats::FIELD_SUBCRIPTIONS_COUNT == $field) {
$this->$subscriptions_count = $cnt;
}
if (!$this->create()) {
$logger->notice(__METHOD__ . " insert $field failed, topic_id=$topic_id");
return false;
}
}
return true;
}
public function incRewardCount($topic_id, $cnt = 1)
{
return $this->increaseStats($topic_id, $cnt, TopicStatsModel::FIELD_REWARDS_COUNT);
}
public function incSubscribeCount($topic_id, $cnt = 1)
{
return $this->increaseStats($topic_id, $cnt, TopicStatsModel::FIELD_SUBCRIPTIONS_COUNT);
}
public function incLikesCount($topic_id, $cnt = 1)
{
return $this->increaseStats($topic_id, $cnt, TopicStatsModel::FIELD_LIKES_COUNT);
}
public function incCommentsCount($topic_id, $cnt = 1)
{
return $this->increaseStats($topic_id, $cnt, TopicStatsModel::FIELD_COMMENTS_COUNT);
}
public function incReadCount($topic_id, $cnt = 1)
{
return $this->increaseStats($topic_id, $cnt, TopicStatsModel::FIELD_READ_COUNT);
}
如果增加某个字段,比如增加1个点赞,那么调用incLikesCount($topic_id, 1),减少一个点赞,那么调用incLikesCount($topic_id, -1)。
现在,看看简化前后的代码,那块更精简清晰便于维护?呵呵,你会发现那个简化前的代码,字数占了本文的一半,不用比较也看出哪个优劣了。
这次代码优化,没有高深的知识,也不用很资深的经验,相信国内大部分开发者都能做到。感觉这是一件事件,开发者有没有想去做的精简一点而已。
(全文完)
(欢迎转载本站文章,但请注明作者和出处 云域 – Yuccn )