一个工作了 6 年的同事写的代码，不看 git 记录我还以为是实习生写的

[garlics]

第一次见到那么离谱的代码，完全没有封装的意思。

        if ($config['theme'] == 1) {
            if ($config['plugin']['PG_SHOPPING_CART']) {
                $tabIndex = [
                    'index' => 0,
                    'collection' => 1,
                    'bestforyou' => 2,
                    'cart' => 3,
                    'mine' => 4
                ];
                $tablist = [
                    [
                        'pagePath' => '/pages/index/index',
                        'text' => '首页',
                        'iconPath' => '/assets/images/home_unactived.png',
                        'selectedIconPath' => '/assets/images/theme1_home_actived.png',
                    ],
                    [
                        "pagePath" => "/pages/collection/collection",
                        "iconPath" => "/assets/images/collection.png",
                        "selectedIconPath" => "/assets/images/collection_actived.png",
                        "text" => "收藏"
                    ],
                    [
                        "pagePath" => "/pages/bestforyou/bestforyou",
                        "iconPath" => "/assets/images/bestforyou_unactived.png",
                        "selectedIconPath" => "/assets/images/bestforyou_actived.png",
                        "text" => "为你优选"
                    ],
                    [
                        'pagePath' => '/pages/cart/cart',
                        'text' => '购物车',
                        'iconPath' => '/assets/images/cart_unactived.png',
                        'selectedIconPath' => '/assets/images/cart_actived.png',
                    ],
                    [
                        'pagePath' => '/pages/mine/mine',
                        'text' => '我的',
                        'iconPath' => '/assets/images/mine_unactived.png',
                        'selectedIconPath' => '/assets/images/mine_actived.png',
                    ]
                ];
            } else {
                $tabIndex = [
                    'index' => 0,
                    'collection' => 1,
                    'bestforyou' => 2,
                    'mine' => 3
                ];
                $tablist = [
                    [
                        'pagePath' => '/pages/index/index',
                        'text' => '首页',
                        'iconPath' => '/assets/images/home_unactived.png',
                        'selectedIconPath' => '/assets/images/theme1_home_actived.png',
                    ],
                    [
                        "pagePath" => "/pages/collection/collection",
                        "iconPath" => "/assets/images/collection.png",
                        "selectedIconPath" => "/assets/images/collection_actived.png",
                        "text" => "收藏"
                    ],
                    [
                        "pagePath" => "/pages/bestforyou/bestforyou",
                        "iconPath" => "/assets/images/bestforyou_unactived.png",
                        "selectedIconPath" => "/assets/images/bestforyou_actived.png",
                        "text" => "为你优选"
                    ],
                    [
                        'pagePath' => '/pages/mine/mine',
                        'text' => '我的',
                        'iconPath' => '/assets/images/mine_unactived.png',
                        'selectedIconPath' => '/assets/images/mine_actived.png',
                    ]
                ];
            }
        } else {
            if ($config['plugin']['PG_SHOPPING_CART']) {
                $tabIndex = [
                    'index' => 0,
                    'cart' => 1,
                    'mine' => 2
                ];
                $tablist = [
                    [
                        'pagePath' => '/pages/index/index',
                        'text' => '首页',
                        'iconPath' => '/assets/images/home_unactived.png',
                        'selectedIconPath' => '/assets/images/home_actived.png',
                    ],
                    [
                        'pagePath' => '/pages/cart/cart',
                        'text' => '购物车',
                        'iconPath' => '/assets/images/cart_unactived.png',
                        'selectedIconPath' => '/assets/images/cart_actived.png',
                    ],
                    [
                        'pagePath' => '/pages/mine/mine',
                        'text' => '我的',
                        'iconPath' => '/assets/images/mine_unactived.png',
                        'selectedIconPath' => '/assets/images/mine_actived.png',
                    ]
                ];
            } else {
                $tabIndex = [
                    'index' => 0,
                    'mine' => 1
                ];
                $tablist = [
                    [
                        'pagePath' => '/pages/index/index',
                        'text' => '首页',
                        'iconPath' => '/assets/images/home_unactived.png',
                        'selectedIconPath' => '/assets/images/home_actived.png',
                    ],
                    [
                        'pagePath' => '/pages/mine/mine',
                        'text' => '我的',
                        'iconPath' => '/assets/images/mine_unactived.png',
                        'selectedIconPath' => '/assets/images/mine_actived.png',
                    ]
                ];
            }
        }

觉得离谱的原因是我现在有个需求，根据$config['plugin']['PG_CATEGORY']，增加一个

[
"pagePath" => "*",
"iconPath" => "*",
"selectedIconPath" => "*",
"text" => "*"
],

发现按照他这个逻辑写简直是个灾难。

感谢60L的老哥帮我发现了个bug

    $map = [
            'index' => [
                'pagePath' => '/pages/index/index',
                'text' => '首页',
                'iconPath' => '/assets/images/home_unactived.png',
                'selectedIconPath' => '/assets/images/home_actived.png',
            ],
            'cart' => [
                'pagePath' => '/pages/cart/cart',
                'text' => '购物车',
                'iconPath' => '/assets/images/cart_unactived.png',
                'selectedIconPath' => '/assets/images/cart_actived.png',
            ],
            'mine' => [
                'pagePath' => '/pages/mine/mine',
                'text' => '我的',
                'iconPath' => '/assets/images/mine_unactived.png',
                'selectedIconPath' => '/assets/images/mine_actived.png',
            ],
            'collection' => [
                "pagePath" => "/pages/collection/collection",
                "iconPath" => "/assets/images/collection.png",
                "selectedIconPath" => "/assets/images/collection_actived.png",
                "text" => "收藏"
            ],
            'bestforyou' => [
                "pagePath" => "/pages/bestforyou/bestforyou",
                "iconPath" => "/assets/images/bestforyou_unactived.png",
                "selectedIconPath" => "/assets/images/bestforyou_actived.png",
                "text" => "为你优选"
            ]
        ];
        $tab = ['index'];
        if ($config['theme'] == 1) {
            $tab[] = 'collection';
            $tab[] = 'bestforyou';
        }
        if ($config['plugin']['PG_SHOPPING_CART']) {
            $tab[] = 'cart';
        }

        $tab[] = 'mine';
        $tabIndex = array_flip($tab);
        $tablist = array_map(function ($v) use ($map) {
            return $map[$v];
        }, $tab);
        if ($config['theme'] == 1) {
            $tablist[0]['selectedIconPath'] = '/assets/images/theme1_home_actived.png';
        }

如果他是这样写的，我估计还不会说什么，最多就默默改了，毕竟一屏也能把逻辑看完，也算逻辑清晰了。

$tabCart =  [
    'pagePath' => '/pages/cart/cart',
    'text' => '购物车',
    'iconPath' => '/assets/images/cart_unactived.png',
    'selectedIconPath' => '/assets/images/cart_actived.png',
];
if (something) {
    $tablist = [$tabCart];
}

他这一屏都看不完一个if块的，真的不能忍。

[afx]

@garlics 那是因为大多数人都这么写，所以他们觉得没有问题。

[northisland]

@Inside 你这个货要是真的天天 review 你 team 的业务代码，你绝对不会表现的这么少见多怪。

[gaorenhua]

代码木有好坏，功能实现才是王道。

[dustin2016]

以代码来评价,本身就是小白的行为
1.一天开几次会?
2.天天加班否?
3.需求疯狂变更否?
4.代码前任的前任的前任哪家去高就了?

一个六年的同事,这样的代码已经离开了器,成就道的水平了

[ashine]

最离谱还是 有 notEmpty() 又有 notZero()，notNull() 这样的封装，看代码才叫混乱，宁愿看多几行代码也不愿去查怎么实现的

[KuroNekoFan]

反正就是 要啥自行车
要么就是 别人都这么搞

[weirdo]

又不是不能用 [手动滑稽

[TuringGunner]

这个显然是因为需求多，没时间重构造成的啊

[dustinth]

很多老程序员急是因为见得多了那些菜鸟程序员过早优化封装, 结果就是还不如不封装; 写代码跟做其他事情一样讲究的是解决主要矛盾, 在我看来这段代码逻辑简单, 也不是什么核心代码, 楼主纠结于此并说很离谱(这段代码确实写的是不怎么样, 我可能也会顺手改了, 但不会太苛责), 那么在面对复杂的核心问题超过楼主的能力时, 这种凡是要抽象封装的理念只会带来错误和难以维护的设计, 危害会比更少设计来得大.
说到 Code Review, 讲一个笑话: 一个人去 update 一个开源代码, update 几行的时候几个核心程序员讨论了半天才 Approve; 当 update 超过几百行的时候十分钟(run 完 unit test)就接受了.

[zoharSoul]

这代码逻辑挺清晰的, 是人都能看懂.

比那强行瞎抽象封装的好多了.

[matthewzhong]

我觉得这个真的要批斗一下才行

[kemikemian]

楼主是差异工作了 6 年居然这么写,还是工作了 6 年只会这么写
不管你工作了几年,老板改一个需求你就要推倒重写的时候,你是否还会坚持花时间去优化代码,让代码更优雅?

[sherlockwhite]

@ashine 同感，我司老板封装的方法，里面就一句查询数据库的，我都要看下是什么，我都吐了。。。

[jiankaikey]

$config['theme'] = 1;
$config['plugin']['PG_SHOPPING_CART'] =1;

$how = 10000000;

$time1 = time();
for($i=0;$i<$how;$i++){
test1($config); //他的代码
}
echo time() - $time1."\n";


$time2 = time();
for($i=0;$i<$how;$i++){
test2($config); //你的代码
}
echo time() - $time2;

别不服气，你的代码慢 10 倍

[newmlp]

我觉得挺好的，清晰易懂

[NoobX]

虽然不算优雅，但不算糟糕的代码。变量名清晰，缩进明了，唯一的缺点可能是长了点？
真正糟糕的代码是变量名函数名乱起、看不懂、到处嵌套在一起，改个 bug 可以改半天甚至无从下手维护的代码

[dustinth]

@jiankaikey 在这里, 效率并不是需要考虑的主要问题.

[northisland]

隔行如隔山。我确实不懂世界上最好的语言。


看起来像是修改一段 json 。。。你吐槽的同事把 json 内容重复了好多遍。日后改动、扩展要重复好多遍。

但是，我对你的同事表示理解。。。


如果你按照书本来，创建大量几乎同样的对象，并且便于日后改动，学过《设计模式》的话，你应该知道要用原型模式或者工厂模式，
写一个 Prototype 类或者 AbstractFactory 类，所有的条件都对应一个子类。

[forgottencoast]

@gaorenhua 103#
这就是瞎扯了。
维护成本才是软件开发成本占比最大的部分。

[zengming00]

简单二字比任何技巧都要好，虽然我不会 php 但这段代码我一看就懂，这段配置文件如果有校验功能那么它一点问题都没有，假设他真的是一年经验重复做了六年编码水平不高但是代码却很清晰

[northisland]

再看了一遍，还是觉得你同事写的没问题。dead simple 风格。

[ytmsdy]

问题的重点是在变量的封装上，如果哪一天“首页”的 icon 要改，那要改好多地方。
判断逻辑上每个人都有每个人的书写习惯，无可厚非！

[loginbygoogle]

不算糟

[ruike007]

这。。没啥吐槽吧。代码工整，逻辑清晰。业务需求问题，人家当初没那个需求啊

[Nicoco]

红利期还在，能允许这样的乐色存在，
红利期不再，这样的乐色就是裸泳被拍死在沙滩上的。

[shiny]

赶紧进来看看是不是我自己的代码，一看不是，又放心的关掉了

[robotdiy]

一般没做过啥有技术难度的，喜欢挑小问题*来寻找存在感。
小问题* 我更感觉是随着业务变化而变化的，再怎么会优化也就是个雕虫小技，无非就是每个人的风格差异而已。

[Kirsk]

码农 没毛病 工程师 不行 维护性是向前兼容 可见预期

[YAHIKO0]

代码重复太多了。

[wfd0807]

笑而不语

[reus]

看这里的回复就知道，说什么 35 岁失业，还是这个行业红利太多了，太宽容了，认为这样的代码没问题的，居然还能混日子到 35 岁？
完全一样的对象，就写成变量，这种基本的技巧都不会？这都算不上封装，纯粹的 don't repeat yourself 而已。基本的原则都抵制，我认为本帖很多人都是不合格的开发者。
这样的人，到了一定年龄就失业，我喜闻乐见！

[baozijun]

@kera0a #56 哈哈,最后的 goto1 笑了

[ifoolish]

吐槽之前最好先在 git 上看看这段代码的变更记录，从中也许可以了解到人家的心路历程

[WilliamYang]

大部分码农的水平都很低，你跟他们说可维护性，他们怎么能理解呢，不喷你的就很好了，有些人还会攻击你代码洁癖呢

[EscYezi]

我觉得封一个简单的函数就可以少很多代码行数，搞不懂楼里怎么有这么多喷的？

[sonyxperia]

人家工资比你高吧。

[darknoll]

工作头两年，我也看不惯这种代码，现在我写得比他"离谱"

[NeoMatrix]

以前看不惯，现在更多的是想问问当时给这位同事多少时间？ 讲个自己亲身经历的事情：之前一直做基础架构相关工作。18 年的时候

[NeoMatrix]

18 年的时候接到了个任务，让一个月内一个人搞个功能还算复杂的 web 后端上到生产环境并且商用（为 xxx 大会做 PR ）。语言不是常用的语言，PRD 过了一周才给出来。最后倒是真的上了线，代码写的跟屎一样，还辛苦后面接手的同学重构了下。问题是：让我两天给你建个桥过河，那只能是简易桥；让我一年给你建个桥过河，情况肯定不一样。

[love]

哈哈看过类似的。
一般是一开始选项少，就用暴力方式手工直接写出结果数组就行了，省得写代码计算了。
直到后来选项越来越多就崩了。。。

[garlics]

@qqjt
@zhangjiafan
@hauzi
@ifoolish
@love
如果他一开始就是这样写的呢?https://imgchr.com/i/0FL7sf

[anxiousPumpkin]

不知道说写得好的人是真这样想的还是抖机灵

既然你做的是这个行业，工作内容就是与别人协同开发，那最起码要负点责任，让别人维护起来不那么难。楼上都是些💩山代码贡献者？

[charlie21]

学习了

[iyangyuan]

曾经我写了一段自以为漂亮的代码，但是需求每天都在变，很多所谓的公共封装都变得难以复用，最后发现还是重复写多次简单，需求再怎么变，逻辑也不乱，因为分开处理互不影响。重复代码并不一定不好，要分项目，分场景。

[zr8657]

你是管理者吗？你的的职责包括维护代码质量吗？把这份代码重构好对你来说比早点下班更重要吗？
我曾经也遇到过类似的问题并且也来发了个贴，后来我跑了

[abersheeran]

请不要侮辱实习生……

[zhuweiyou]

就我一个人觉得挺好的吗?
虽然你可以优化得短一点,但是面对复杂需求的时候,还是他的比较清晰明了

[Niphor]

从头开始写和 review 代码，心态是不同的

说不定这段代码是几个越黑风高的 996 晚上，和产品吵了十几遍出来的....

[back0893]

你把你的代码贴出来比比

[Niphor]

P.S. 周末的瓜 真香

[tairan2006]

就改成表驱动嘛…

[hauzi]

@garlics 那确实是懒省事

[samgentlem]

我竟然感觉这段代码逻辑异常清楚。。。我是不是没救了

[i4oolish]

too young too simple. 别只订着别人展现出来的代码看，要看问题背景和最后结果。

[soulmt]

带点脑子的实习生 也不会这么写吧，难道不是轻微的封装一下更方便快捷吗？？？还是说这里大部分人粘贴复制习惯了改不过来了？请问封装一个映射关系能花多久？在动刀之前就把尽可能的配置写好，最后就算改，也只是改一改配置和映射关系吧？到底是一改改 4 次好还是改一次，维护映射好？

[0703wzq]

这种代码长期下去 if 就会越来越多，每个 if 都要包含一段其他 if 已有的代码， 一旦要增加需求，就是个灾难。竟然有那么多人觉得没毛病....

[sugars]

@Niphor 或者那晚加上还被戴绿帽了，真是心酸啊，脑海中也只剩 if else 了

[onebyone]

你觉得不好 改成自己觉得好的就是了 何必这样 显得自己比较优秀？

[KuroNekoFan]

我说有的人是不是没搞清楚什么叫复用什么叫封装啊

[leafre]

你们公司是以代码行数做 KPI 的吧

[lhx6538665]

你都能看懂了，说明代码很好理解，是好代码

[xuewuchen]

代码挺清晰的，缺点可能就是可维护可扩展性差点
对于我个人来说，会把经常需要维护的一些功能做成泛型类。反正可能写了就不咋动的就咋简单方便怎么来了。
在于我看来代码没有所谓的垃圾代码，因为所有没有核心数学模型算法的只是单纯功能实现的代码都是垃圾代码，一个培训 1 个月就能 CODER 的代码都是垃圾代码，什么界面布局，什么按钮命名，什么数据查询。。
啊。。不好意思。负能量了

[yanghuaqiang]

看了你的附言，更难懂了，原代码在逻辑上更加清晰，只是不够优雅。
要我优化的话：
- 首先定义一个默认返回的数组
- 保留原逻辑 if else
- 再在默认数组中做操作 unset 函数就够了
想说的是：不要为了优化而优化。

[a719031256]

@EscYezi 楼主应该没分清楚作业跟工作的区别

[skinny]

虽然不是程序员，不过看一圈下来，算是理解为什么很多程序员 30+就被淘汰了，还真不全是因为不够资本家压榨，就是活该。

[jokeqf]

上面的评论看完了，没人对代码里的硬编码有疑问吗？特别是如果没有注释，可维护性极差。

[ylsc633]

清晰明了 看看楼上多少不是写 php 的都能看懂,说明确实很清晰

唯一不太好的就是 if 太大了 if 太大了会影响视觉判断 这个 else 是对应哪个的

可解决办法两个

1. 大方法上面写清注释

比如:

// 如果 A = B
// 如果 A > C
// 或者 A <= C
// 或者 A
.....


2. 把 if 里大方法摘出来, 再放到 if 里 这样 if 特别清晰明了,后者修改也知道需要修改那块

比如 a = xxx b = xxx
if 条件
a
else
b



我个人觉得 web 应用千万不要过度封装, 性能不好再说, 后者维护也真的麻烦, 看了半天才能看懂啥意思,还特别特别容易出现 bug

[Wincer]

“一个工作了 6 年的同事写的代码，大家都来看看自己和他的差距在哪”，楼主要是这么起标题，楼里大部分说这代码好的人估计就会开喷这代码 xx 了。

[xpfd]

看了一圈下来发现很多年轻的小盆友没被现实毒打过, 还理解了为啥 30+的程序员就被淘汰,哎,年轻真好,无知无畏的,挺好的

[sevenzhou1218]

我只想说写代码|打补丁和 review 代码心态就不一样.

[lepig]

KPI 下的产物 不奇怪

[anerevol]

静态的看待这个问题，同事的代码是有点啰嗦
但是事情发展一般都是动态的

[
'pagePath' => '/pages/mine/mine',
'text' => '我的',
'iconPath' => '/assets/images/mine_unactived.png',
'selectedIconPath' => '/assets/images/mine_actived.png',
]
要不要把这个抽出来封装这个事情还真不好说 也许只是恰好各种条件都是这样的 没有更多的上下文条件不好定论

等楼主见过运行了 5678 年，换过 345 波人的项目代码，肯定会觉得现在同事的代码好像也不算非常糟糕了

[fengmumu]

@kera0a 哈哈哈哈哈哈

[kanepan19]

你以为大厂里业务代码有好多少呢 .
除了核心代码, 很多边缘的业务代码比这个 ifelse 还要多

[afx]

@soulmt #155,你跟复制粘贴党谈维护，人家跟你谈工资啦。

[afx]

@jiankaikey 要这么说的话为什么不把键盘上除了 0 和 1 之外的键都抠掉

[kidlfy]

我竟然看懂了

[ooppstef]

只希望, 以后 lz 在未来的工作中, 任何时候都坚持这个帖子里面的态度,任何时候都避免所谓的"垃圾代码".

以后别活成自己讨厌的人就好.当然,如果真能做到,lz 成就应该会很大很大.

代码好坏不谈,任何人都会因为各种外部因素写出质量低劣的代码.但我并不认为,可以通过一段代码,做出如此帖子的评论.

[blackboom]

刚工作的时候我也喜欢批评老代码，顺带损一下老员工。工作几年后我开始明白某些项目能跑就行了，目前依旧对代码质量有要求，不过分主次和轻重了。

[djoiwhud]

完蛋了，业务代码碰到了自我感觉良好的洁癖码农。你所谓的新需求，在以前是不存在的。为不存在的业务设计代码，分分钟就背一个磨洋工的锅。

既然不能忍，赶紧辞职。

心态有问题要承认。

[shenjinpeng]

首先你不说这段代码曾经改过几次, 或者是某个深夜收到需求加班赶出来的 . 评价代码好坏离不开实际的业务需求 . 把所有选项拆出来再组合的前提是格式固定, 没有定制化需求的前提 . 最后, 说实话, 楼主封装后的代码效率低下了很多倍, 内存占用多了 n 个数量级 . 封不封装取决于后面的需求还会不会变更, 是否还会增加 ifelse , 团队代码统一风格, 项目的大小以及投入的时间成本 .

[no1xsyzy]

不是挺符合函数式的么（
$tab[] = 这种语法是指 append 或者 prepend ？
所谓封装应该是写个 make_tablist 函数，根据表项 list 和通过 context 或者 thread cell 传递的 theme 来构造 tablist

[lovecy]

楼主优化过后的代码简洁了，但是对逻辑要求更高了，至少新人一眼能看懂原代码，但要花点心思读你的代码。
#181 楼说的挺好

[mrkelly]

刚入行半年的时候，看到前辈的代码，也很困惑，主动找了领导聊。
当时领导说：「有时候，越简单、越易读越好，适合团队协作」。

8 年后的今天我终于明白了，确实如此.....

经验是：

只要架构稳定，局部代码，永远都会迭代，可读性>魔幻性，先快写；
但是，整体架构，必须要想得长远。

[flicking2015]

正常，我也工作快 6 年了，和我最近写的前端代码差不多（ ps：搞后端的）

[AJQA]

工作 3 年后从自己做外包开始 重新审视什么是 code review：
一群人在投影机前 建议代码要怎么写 一行一行一块块下来 我的总结就这：没产生实际价值 宝贵的生命被浪费了

[lucifineil]

清晰易懂，真封装了，六年后能不能看懂都是个问题

[tanjian]

没毛病，用的少，我也懒得封装，用的多的我才想去封装一下！

[soulmt]

@mrkelly 没有远见的可读性就是对可读性最大的不尊重，试想这段代码目前只有 4 块，将来业务逻辑复杂，变成 10 块，或者场景变成 20 个，甚至更多，请问，还有可读性可言么？ 屏幕竖起来滑半天可能才能窥探这个配置信息的逻辑是什么

[Nostalgiaaaa]

$config['theme'] == 1
这个 1 好歹写个常量吧，真较真槽点还是有的。

[securityCoding]

可读性排第一,这段代码并没有什么问题

[soulmt]

@jackrelative 等你维护了老代码就知道了， 希望你维护了之后，或者遇到类似代码跳脚之前，赶紧离职

[wangyzj]

其实这是一个权衡的问题
相比这么写
过度封装更影响阅读
所以权衡好就可以了，毕竟有些业务真的复用性很低

[Nostalgiaaaa]

或者 tablist 每个元素中自带属性
[
'pagePath' => '/pages/index/index',
'text' => '首页',
'iconPath' => '/assets/images/home_unactived.png',
'selectedIconPath' => '/assets/images/theme1_home_actived.png',
'type' => 'index',
]
list 本身就有元素顺序的信息在里面，加上类型的话可以不传 tabIndex 。你写一个 map 就行了，不同的 if 条件构建不一样的 tabList，
但是我觉得现在这个样子不是不能忍的。有了新需求比如新增了一大波 tab 类型，或者 tab 类型需要个性化时候重构就行了。就现在的话有空改没空不改。

[soulmt]

@afx 害，是我想太多。

[easymbol]

工作嘛，实现为主，真正有意思的代码是开源的

[bk201]

提前做些无关紧要的事情没啥意义。

[lsj8924]

没有对比就没有伤害，请写出你自己优化后的代码，让大家看看到底哪个更好。

[javapythongo]

我觉得优化是有一个过程的，如果最开始业务就挺简单，就简单写，当业务复杂起来时，再对代码进行优化

[sunziren]

新的一页，新的开始