那些有坏味道的代码
2013-11-27 19:33
169 查看
最近每天早上上班的第一件事情就是把昨天写的代码重构优化一下,以前没弄过,现在发现这个过程真是非常爽的。看着代码一点点变好,还是很不错的感觉。
最经常遇到的一些坏味道这里列一下:
这里的问题就是代码嵌套太多层了,如果foreach里面有很多东西的话更难看
改成:
其实上面一个代码一定是最符合人类思维才会这样写的,而且不得不承认很容易会写出这样的代码
后面的那个代码把阅读顺序稍微修改了下,不大影响阅读,也能优化代码,不错的。
上面一段代码,当去Suggest中获取数据失败的时候,就去Search中获取ID,但最后实际都是做一样的操作doAction
那么可以优化:
首先由于php是若类型的,所以需要养成的好习惯:
1 一个封闭函数应该返回什么类型的数据就返回什么类型的数据
比如这里的getSearchIds的函数,如果内部原先是期望返回array的,那么如果里面的数据为空的话,请直接返回array(),不要返回null或者false。
2 一个封闭的函数最开头需要做预防性的行为
比如这里的doAction($ids) 最先需要对ids如果为空数据的话如何处理
这里的getSearchIds和doAction应该是这样的:
可以看到,这样做如果ids是array(), 会被过滤掉,即使没有过滤,foreach也不会发出warnging。
一个controller中的逻辑大概是:
step1: 获取参数
step2: 根据参数获取数据
step3: 将数据重新组装返回
如果你的php是一个接口,那么这个第三步的东西估计是天天需要做的
所以治本的方法就该是学习rest风格,先定义好一种通用的数据,然后做个通用的映射逻辑(这个是可以做的,因为数据库的格式其实是固定的)
但是呢,我的观点是不能“滥通用”。意思就是不能每每看一个东西就认为是通用的。
比如有个观点“所有配置文件都应该放在统一的地方”。
这个我认为是不必要的,只有使用一次的配置数据就应该放在使用的地方,这样能保证代码的可读性
比如这里的:
config这个变量,有人就会习惯放在其他代码里面,或者通用。但是实际上,这个几乎是永远不会再用,如果把它放到一个所谓“通用”的地方,读代码的话会出现阅读障碍。
反而是不美了。
最经常遇到的一些坏味道这里列一下:
嵌套太多
if (!empty($data) { if (is_array($data)) { foreach($data as $item) { // Do something } return $data; } } else { return null; }
这里的问题就是代码嵌套太多层了,如果foreach里面有很多东西的话更难看
改成:
if (empty($data) || !is_array($data)) { return null; } foreach($data as $item) { //Do something } return $data;
其实上面一个代码一定是最符合人类思维才会这样写的,而且不得不承认很容易会写出这样的代码
后面的那个代码把阅读顺序稍微修改了下,不大影响阅读,也能优化代码,不错的。
函数返回数据的验证
if ($ver >= 10901601) { $suggestIds = SuggestModel::getSuggestIds($word); if (!empty($suggestIds)) { $this->doAction($suggestIds); } else { $searchIds = $this->getSearchIds($word); if (!empty($searchIds)) { $this->doAction($searchIds); } } }
上面一段代码,当去Suggest中获取数据失败的时候,就去Search中获取ID,但最后实际都是做一样的操作doAction
那么可以优化:
if ($ver < 10901601) { return; } $ids = array(); $ids = SuggestModel::getSuggestIds($word); if (empty($ids)) { $ids = $this->getSearchIds($word); } $this->doAction($ids);
首先由于php是若类型的,所以需要养成的好习惯:
1 一个封闭函数应该返回什么类型的数据就返回什么类型的数据
比如这里的getSearchIds的函数,如果内部原先是期望返回array的,那么如果里面的数据为空的话,请直接返回array(),不要返回null或者false。
2 一个封闭的函数最开头需要做预防性的行为
比如这里的doAction($ids) 最先需要对ids如果为空数据的话如何处理
这里的getSearchIds和doAction应该是这样的:
function getSearchIds($word) { if (empty($word)) { return array(); } //DoSomething return $ids; } funciton doAction($ids) { if (empty($ids)) { return false; } // DoSomeThing foreach($ids as $id) { // Do SomeThing } return true; }
可以看到,这样做如果ids是array(), 会被过滤掉,即使没有过滤,foreach也不会发出warnging。
返回格式映射
php的controller中做的最多的是什么,一定是会说是返回格式的映射一个controller中的逻辑大概是:
step1: 获取参数
step2: 根据参数获取数据
step3: 将数据重新组装返回
如果你的php是一个接口,那么这个第三步的东西估计是天天需要做的
所以治本的方法就该是学习rest风格,先定义好一种通用的数据,然后做个通用的映射逻辑(这个是可以做的,因为数据库的格式其实是固定的)
通用代码
通用代码是经常遇到的一个问题,显而易见,如果一份代码经常使用,那么就应该把它封装起来,然后做成一个通用的。但是呢,我的观点是不能“滥通用”。意思就是不能每每看一个东西就认为是通用的。
比如有个观点“所有配置文件都应该放在统一的地方”。
这个我认为是不必要的,只有使用一次的配置数据就应该放在使用的地方,这样能保证代码的可读性
比如这里的:
function getManualAppsIds() { $config = '/data/conf/test.json'; $content = file_get_contents($config); $manuals = json_decode($content, true); return manuals; }
config这个变量,有人就会习惯放在其他代码里面,或者通用。但是实际上,这个几乎是永远不会再用,如果把它放到一个所谓“通用”的地方,读代码的话会出现阅读障碍。
反而是不美了。
相关文章推荐
- 那些令人喷饭的代码注释:仅以此代码献给...
- 代码坏的味道22:过多的注释 (Comments)
- 科幻大片中那些牛X代码真相
- 从抽象到模式——面向对象之旅(二)、代码的坏味道
- 代码的坏味道【2】
- 要你命三千:老代码中的那些坑
- 《重构--改善既有代码的设计》--代码的坏味道(3)
- 那些令人喷饭的代码注释
- 21种代码的“坏味道”
- 代码中的坏味道
- 【整理】【代码的坏味道】过长参数列(Long Parameter List)
- 读【重构】读书笔记之一 代码的坏味道
- 代码的坏味道
- 要你命三千:老代码中的那些坑
- 改善代码的坏味道,很苦的一段代码
- 写代码写至最有面向对象味道
- 那些出现在电影中的程序代码
- 让开发自动化持续重构 --使用静态分析工具识别代码味道
- 22种代码味道(Martin Fowler与Kent Beck)
- 代码的坏味道之二——译自《重构》