好像不自觉就写了难以维护的代码 ...

2014-11-29 21:08:16 +08:00
 Sunyanzi
今天遇到这么个事儿 ... 我自己也拿不定主意 ... 想来想去还是发帖子了 ...

起因是这样 ... 我因为不能忍线上的支付宝相关代码是官方的示例 SDK ... 所以整个重构 ...

重构完毕的代码里有这么一段 ... 用以一站式判断 PC 和 WAP 的返回值 ...




我自己觉得写的还蛮顺的 ... 延着思路就写下来了 ...

但今天在讲我重构之后的代码时 ... 大家的反应是「看不懂」「不敢改」「维护不了」 ...

我就有点心虚 ... 虽然可以强权镇压但我终归还是想知道更多人的看法 ...

诚然我可以把这段代码拆成分散在三四个文件里的五六个类 ... 然后用非常华丽的技法加载 ...

但我个人觉得那样才是没必要 ... 更加占用资源而且更加难以维护 ...

最近 V2 上也在讨论什么样的代码简单易懂 ... 赶着这阵子正好问问 ...

反正我个人就是觉得谁如果觉得我这段代码晦涩只能说明他自己 php 水平不过关 ...

是我错了吗 ...
9822 次点击
所在节点    PHP
79 条回复
jarlyyn
2014-11-30 11:09:44 +08:00
支付宝的php代码我也重构过。
不过处于业务需要,是建了支付类的子类,然后所有功能尽可能的拆细了,主函数控制流程。
需要测试只要测各个子函数,需要检查流程看主函数就可以了。
你的代码风格是我所不喜的风格。
还有访问的url等常量我个人会放在文件头的一个变量/类属性里。方便以后调整。直接写代码里不容易调试,还容易让修改的人该错。
jarlyyn
2014-11-30 11:13:42 +08:00
还有从我的角度来说。代码更重要的是可读,可修改,可测试。
可读是知道整体逻辑,知道流程,而非怎么去实现。实现与否不该靠读出来的。
可修改是只任何需要修改的地方应该只牵涉到几处代码片段,而非整个代码。
可测试是和可读对应的。
逻辑,功能靠读。
实现不应该靠读,靠测试。
所有走一个逻辑没问题,跑一次单元测试没问题,那就ok了。
qaulau
2014-11-30 11:14:37 +08:00
三元运算里套三元运算,关键是连个括号都没有,这样的代码也就自己看着爽
loryyang
2014-11-30 11:19:30 +08:00
我觉得写的不好,首先所有if后面都要加{},这是代码规范。然后一行代码不可以写这么长,要想办法做简化。静态的string用常量来表示,比如你的正则表达式。好处:一是方便复用和后续修改,二是人肉看更直观(因为有命名)。然后函数嵌套的太多了,宁可多设置几个变量或者多抽几个函数,不要把代码揉成一坨。不要说是否看得懂,看到这种代码,没几个愿意认真看的。
scys
2014-11-30 11:21:31 +08:00
挺好懂的,逻辑堆在那里罢了。
你同事比较懒,如果一眼看不懂,基本他们才懒得动。
就这么个理解,嗯嗯。
zwdsix
2014-11-30 11:31:08 +08:00
镇压吧,多爽。
lizheming
2014-11-30 11:39:55 +08:00
说不上难懂,不过一眼能看出来估计有压力吧...反正那个三元混搭Style我是看了两遍,主要是在找:在哪里,2333...不过怎么说呢,我可是鸭子大大的脑残粉,怎么能说鸭子大大写的代码不好呢!恩哼!
cnallenzhao
2014-11-30 12:04:16 +08:00
我是觉得$tmp 这种命名方法太反人类了
abcfyk
2014-11-30 12:23:02 +08:00
最后一个嵌套的三元运算看得我都醉了。。

http_build_query($parameter)) .
$this->_config['key']
拼接字符串时从.号处换行真的好吗。我书读的少。不要骗我。。
还有函数变量里用三元运算符。。还是嵌套的三元运算符真的好吗。。
看来LZ还没意识到其他同事反应的不是你封装的问题,而是 代码风格的问题。
写代码就像写文章一样诚然你可以在一句话里把一整篇文章打完并用了很好的修辞手法和换行但是如果你没有适当的介绍和断句那么不就是像我现在一样咯里吧嗦的打了这么多字但是看起来还是很费力而且不讨好吗要知道好的代码就像一篇好文章能如沐春风般在悄无声息间让读者不知不觉便顺着你的思路如
同文思如泉涌般喷薄而出不由得抚掌而叹好文好文虽文字浅显却立意深刻意味深长的缘故啊。
AlanZhang
2014-11-30 12:30:25 +08:00
没做过支付宝的,但是看这一段代码,还是蛮意大利面条的,分得不够细,代码自身不大expressive.
Gn
2014-11-30 12:32:24 +08:00
楼主写得很妙啊。2333

你是我见过的唯一一个用三元运算符最后还返回true和false的人才啊。2333

return a;
非得写成 return a ? true : false;

return a && b;
非得写成 return a ? b ? true : false : false;

人家看不懂一定是因为你没有拆分成几个类然后用你华丽的技法加载嘛。
waytoexploreOO
2014-11-30 13:09:15 +08:00
qq
zhengkai
2014-11-30 13:23:54 +08:00
这代码不能再糟了,你要在我们组可能会被石刑……

不过对于年轻人这是必由之路,多写代码来提升自己吧(我们公司有个岁数很大的老员工,写 PHP 的,动不动就写 $foo == false 或者 $foo == "" 或者 if (is_array($a) && count($a) == 0) 这类东西……)

以及要看《代码大全》《程序员修炼之道》
crossmaya
2014-11-30 14:32:51 +08:00
楼主是来现英文注释的吗。。。
Sunyanzi
2014-12-01 15:56:46 +08:00
吓 ... 周末睡了一天没上网 ... 回来就发现这么高的楼 ...

我在 V2 有史以来的第一高楼居然是挂自己得到的 ... 还真是悲伤呢 ...

答读者问时间 ... 恐不能面面俱到 ... 如有不周还请体谅则个 ...

@RIcter php 有 PSR ... 不过我讨厌那个规范 ...

@freefcw 谢回复 ... 我想想从哪里说起 ...

关于为什么把 notify 和 return 放在一起 ...

只是因为这两种方式都是支付宝的回调 ... 而且数据结构类似 ...

以及我其实没太看懂你的修改建议 ... 求详细 ...

至于一行代码只有一个功能这点我认同 ... 而且我的那段代码里也没有犯多义性错误 ...

一个函数只有一个功能什么的 ... 也就是函数重载是不能用的咯 ...

@kankana 谢回复 ... 我也不能接受太长的一行其实 ...

在我那张图上最右边有条细细的线有注意到吗 ... 对我而言那条线就是不可逾越的存在 ...

很莫名其妙的坚持我知道 ... 奇怪的强迫症 ...

所以我会很注意不使用过深的语法结构 ... 以及单行超长的时候用换行缩进将其拆分 ...

@miniwade514 谢回复 ... 既然在写的「当时」觉得简单 ...

在不扩充功能的前提下它一定就是简单的 ... 那么为什么还要将其重构掉呢 ..?

换言之 ... 为什么要让代码适应团队而不是让团队适应代码 ..?

@nine 谢回复 ... 我也知道这样会更清晰 ... 但这样就超长啦 ...

参考我上面说不越线的那一条嗯 ...

@kmvan 谢回复 ... 虽然看起来代码一大坨 ... 但我的逻辑有一团糟吗 ...

求脱开代码来解释逻辑糟在哪里 ...

@xylophone21 @RemRain @crossmaya 谢回复 ... 关于注释的问题我一并说了吧 ...

我这人在写代码的时候习惯于写些文法不通的废话 ... 也就是代码里的注释部分 ...

这并不是为了提供什么信息 ... 只是我在思考下一步的时候给手上找点事做而已 ...

不让我写这些废话的话我是无法把代码写下去的 ... 就像转笔一样只是一个辅助动作 ...

@yuxing1171 @Gn 谢回复 ... 我之所以没用 && 也是为了提高可读性 ... 但好像适得其反了 ..?

@otakustay 谢回复 ... 受教 ...

@cnallenzhao 谢回复 ... $tmp 那个确实是偷懒了 ...

但只在单行可用 ... 定义和使用都在一行 ... 下一行就被销毁了 ...

@abcfyk 谢回复 ... 嗯我喜欢这个回复风格 ... 真的 ...

@zhengkai 谢回复 ... 为啥会被石刑啦 ... 地球就是绕着太阳转的哼!
freefcw
2014-12-01 17:23:46 +08:00
@Sunyanzi

我真的不知道怎么说了,很有耐心的给你分析了很多


对于两种返回方式分割成两个函数调用,这样代码会更可读(感觉上)。数据结构类似不是放一块的原因。不过是两个不同的方式而已,你就要弄的这么复杂,让人要停下来前后看才能看懂

payinfo提取为一个类,这样可以把很多相关的功能放到一起,不是更清晰明了?
$notify_id这一行拆分,难道不应该放到payinfo这个类里面去?明显是和payinfo有关的信息

unset这是蛋疼事情,如果后续变动怎么办?

ksort的功能应该和md5这个都放到payinfo的一个方法里面么

return里面到底执行了多少个功能呢?典型应该拆分的地方,一句话做了这么多事情
那个http 请求,不应该是一个单独的函数的功能么
那个签名,如果后续变了怎么办?

唉,不能不感慨下,能写出这样的代码的人,我只能说脑容量一定很大,或者是我等智力有所不及……不然怎么会花这么多人绞尽脑汁分析这段代码呢
freefcw
2014-12-01 17:57:59 +08:00
@Sunyanzi


最近在读《代码整洁之道》,其实正好符合你的这个问题,建议对照着核对一下
beaaar
2014-12-01 22:14:38 +08:00
@mcfog +1
Sunyanzi
2014-12-04 04:31:21 +08:00
嘛 ... 虽然可能已经没人看了 ... 但我还是说一下最新进展好了 ...

顶楼的代码我自以为在「可读性」与「简洁性」之间找到了一个平衡点 ... 看来做得还是不够 ...

于是我又把它重构掉了 ... 去掉了所有三元嵌套只为增加可读性 ...

顶楼的代码就是孤本了 ... 挂在这里做个纪念好啦 ...

这是一个专为移动设备优化的页面(即为了让你能够在 Google 搜索结果里秒开这个页面),如果你希望参与 V2EX 社区的讨论,你可以继续到 V2EX 上打开本讨论主题的完整版本。

https://tanronggui.xyz/t/150282

V2EX 是创意工作者们的社区,是一个分享自己正在做的有趣事物、交流想法,可以遇见新朋友甚至新机会的地方。

V2EX is a community of developers, designers and creative people.

© 2021 V2EX