一个工作了 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块的，真的不能忍。

[tcpdump]

起码挺工整 ，占楼，看楼下大神指点

[fffflyfish]

你懂啥，不把 baseline 打低一点怎么优化啊

[wizardoz]

你以为他是手写的，其实他可能是用模版生成的。

[MuscleOf2016]

咋优化？提取公共写常量？

[tabris17]

都写 PHP 了，还封装啥呀，先完成 KPI 才是正道

[raincode]

起码变量名挺规范 ，占楼，看楼下大神指点

[oukichi]

你可以优化一下然后贴出来，这样不但提高自己而且帮助别人。毕竟吐槽大家都会，但解决问题的人就少了。

[warlock]

如果是模板生成 那么应该加上 Code generated by xxxxx. DO NOT EDIT. 避免误会

[zhoushushu]

我也经常这么写，过度封装，接手的人还得琢磨。

[futou]

#7 +1，等待优化结果

[7654]

看不爽有空就给他改了啊

[victor]

@zhoushushu +1

[sunznx]

@tabris17 这都能黑，你是有病吗

[eGlhb2Jhb2Jhbw]

有些单引号，有些双引号。有些最后元素带逗号，有些最后元素不带。强迫症就很难受。代码除了长点，问题不是很大，也比较容易阅读（当然与业务有一定关系），感觉没你说的那么严重。

[jadehare]

这可太工整了，也就重复代码太多，感觉你还是年轻了，没见过嵌套几层的 if

[revalue]

#7 +2，等待优化结果

表格驱动也不大行

[ingdawn]

坐等大佬封装方案

[haohappy]

起码挺工整 ，遇到事逼了

[RangerWolf]

楼主，把你优化之后的发出来让大家学习学习？

[garlics]

@fffflyfish
@oukichi
@MuscleOf2016
因为要改加些功能，所以我整理了下这段代码，个人认为看起来逻辑清晰很多，也希望有大神有更好的方法改
```
$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);
```

[xpfd]

写的挺好的啊?逻辑清晰,易读,易维护,虽然我不会 php 但是也能快速抓住逻辑,反而是一些年轻小伙为了炫技用这个晦涩的方式实现让人很头大,rewiew 的时候经常要转换理解一下,等到过一段时间自己去看自己写的代码反而迷糊了,这样的见得多了

[qqjt]

问题不大，很可能的情况是，一开始就一种菜单，之后判断条件多了，慢慢出现了多种情况的判断。
屎山不是一天写成的。

[cxxlxx]

就算写的很烂 实习生也不背锅

[northisland]

有点慌，我也经常写这种代码。。。


2 个判断条件，对应 3 个条件。直接这么写。有毛病么？

[ritaswc]

其实通俗易懂也是个有点哦，我做讨厌在业务代码里面炫技了

[a719031256]

过度封装不好，如何简单明了的表达意思就好，业务代码讲究效率，谁 tm 的在乎好不好看，能 10 分钟写完的东西没必要花一个小时去弄

[nicebird]

改成表驱动就行了吧。

[iFlicker]

可能绩效考评参考 git 代码提交行数吧

[foMM]

有没有哪位可以说说如何在回复里面贴图和贴代码块？ Markdown 很明显是不支持的

[qq976739120]

逻辑很清晰,接盘他的代码我不会抗拒

[Sharuru]

这种东西优化顶多把 URL 路由先预设好，然后直接 tablist 里引用。
但是如果碰到一个特别的业务，然后路由对象没接口的话，改自定义值就会头大。

业务代码写多了，最后还是觉得这种平铺直叙式的写法舒服，不要考虑什么优化，糙快猛干就完事儿了。
早点堆完早点摸鱼。

白衣老人表情包.jpg

[kanepan19]

我觉得没毛病,
瞎猜楼主的吐槽的原因是不是 ,对人不对事了

[garlics]

那么多人说写得好我还挺意外的，目前有个需求要按照判断加一个菜单，按照他这个逻辑，判断会变成 8 个，代码直接又长一倍。

[binjoo]

说不定人家 KPI 是算代码行数的。

[eGlhb2Jhb2Jhbw]

@garlics #20 你咋也是单引号双引号混用，末尾逗号爱加不加。PHP 都这么写的吗？

[zhangjiafan]

什么是事儿逼，你就是事儿逼，elseif 不是一天写出来的，而是隔了几天老板开始 else if 的，并且是晚上八点半冷不丁来一句加个需求，这个时候，你会继续 elseif 还是会想优化代码，然后优化几小时吗？从你发帖到优化花了 15 分钟，有这 15 分钟早点回家老婆孩子热炕头不香吗？

[ginjedoad]

这个是要早日起代码行数？保证占领版本库的代码百分比？

[wangbudong]

我还觉得写得挺好的

[KyonLi]

@foMM gist

[xilixjd]

@garlics else 里，tablist 的购物车需要换个 icon，雪崩

[tabris17]

@sunznx PHP 就是为了快速开发，什么都想着封装一下才是有病。

这里的代码明显就是当作 DSL 使用的，还封装个毛线

[Joexjx]

这代码只是看起来不高级，实际比较通俗易懂，没必要为了看上去高级，就对 ifelse 等有深仇大恨之类的

[Immortal]

除了$tablist 没提出去复用,和 theme == 1 这种没有可读性的代码,其他都中规中矩没问题

[sagaxu]

你以为他是手写的，可能他是脚本生成。

然后，老板偷偷看新增代码行数的时候，他产出远比你高。

[northisland]

作者喷的是
if cond1
if cond2
.... // 处理 1
else
... // 处理 2
else
if cond2
... // 处理 3
else
... // 处理 4
endif


请问怎么优化？这样把嵌套 if 条件写在一起，会扁平一点？

if cond1 and cond2:
.... // 处理 1
elif cond1 and not cond2:
... // 处理 2
elif not cond1 and cond2:
... // 处理 3
elif not cond1 and not cond2:
... // 处理 4


楼主说一下优化思路呗 @garlics

[mlxj]

贼鸡儿清晰💯分

[garlics]

不知道为什么不能 append，说写得好的各位，现在有个需求，根据$config['plugin']['PG_CATEGORY']，增加一个
[
"pagePath" => "*",
"iconPath" => "*",
"selectedIconPath" => "*",
"text" => "*"
],
请告诉我怎么加？

[songyijie]

@garlics 所以会不会一开始也没那么多判断, 后来需求加的

[luhe]

我一直以为，封装是别的地方也需要用到才封装，一个地方封来封去的感觉有点浪费时间...（可能说得不对，大佬可以指点一下...

[baosiqing]

写出这样的代码为啥还有觉得好的？我见过不止这样写，还不会格式化的，看他代码经常要研究某个括号是和哪个一起的

[leo108]

确实比较惊讶这么多人可以接受这样的代码。

楼主的优化方案也不算过度封装，实现成本也就几分钟，即使未来有个别特殊需求也能简单地兼容。

[hauzi]

应该不是一次写完的，功能都是慢慢堆砌的，你觉得到了需要优化的地步，就可以优化，这就是程序开发的正常过程。

[leo108]

有些人说是工作 x 年只不过是一年的经验用了 x 年

[wupher]

如果是我的话，可能会抽象成对象，然后把现有数据转换成一个静态对象，或者直接丢到数据库表 /Cache 中。

但我觉得 3 楼说得可能更在理，没准儿这代码就是他用数据库或者模板直接生成的。

[myon]

冗余代码太多，如果有 review 的话过不了。好在逻辑清晰，我不会 php 都能看懂

[tsingke]

坐等楼主贴出来，自己优化后的代码，那些不贴自己的代码，光喷别人代码的人，一律当做傻逼处理。

[steven_yue]

这不就是优化后的结果吗？你写成函数，包成类，最后编译器还不是给你打开了。兄弟是高手

[kera0a]

“第一次见到那么离谱的代码” ？？？

楼主可能💩山见少了，你贴的代码一眼就看明白了，逻辑简单和他处也没啥耦合，这种代码不会出什么问题。
不能说离谱，只能说不优雅

真正离谱的代码是
1 你看都看不明白
2 等你看明白了，你也不敢改
3 等你捋完了敢改了，你也不能确保会不会有 bug
4 产生 bug 了，你也不知道是什么原因
5 知道原因了 goto 1

[ll1615]

我觉得楼主的优化就很好，有些人就是喜欢复制粘贴

[myon]

另外你优化后的代码有 bug，首页的 selectedIconPath 是跟随 theme 变化的，看到这点我大概理解他这么写的原因了，要提取出来代码会比较绕

[BeFun]

通俗易懂，我觉得挺好，至少自己三个月后再来看，还能一眼看懂

[stach]

不管是实习生, 还是资深工程师代, 码逻辑清晰是业务系统最关键的地方. 当然楼主可以优化代码组织方式, 封装一下, 我相信改起来没有任何压力 (这代码蛮清晰的嘛, 没有暗坑).

不过要提醒的是, 楼主也不要过度封装, 没有哪种模式是普适的, 过度封装的副作用更大.

[chenyu0532]

记得前几年有一个巨火的手游，用 cocos2d 写的，被别人破解了。发现里面极其多的 if elseif，一层层的包着，看着都头大。。但是人家就是很火。。
个人认为，如果为了自己的代码精进，那相信在座的各位的代码都会优化；如果侧重点在业务上，就可以怎么快怎么来，自己和别人看得懂。
以前我也觉得代码优化最重要。。。但是慢慢觉得业务 /策略 /想法才是最重要的。。

[tydl]

工整、清晰，一目了然。不建议封装~~

[dustinth]

封装意味着多一层抽象, 会多一层心智负担, 所以需要有代码清晰度的提高或者减少重复来作为补偿(比较明显的结果就是封装后代码行数明显变短). 楼主同事的代码没什么大问题, 更谈不上"离谱", 经过优化后代码并没有明显的代码量的降低.

[coderljx]

@kera0a 哈哈，很现实了

[Varobjs]

这不算什么，毕竟 if else 嵌套的只是简单赋值；

你是没见过 很多个 if else 结构，每个包含数百行业务逻辑

然后各个业务逻辑 80-90%是相似的，

[a194259440]

面向过程得通俗易懂，就能说明那代码还挺好得？面向对象学的啥，我更赞同这代码不是一次写成得，导致多层嵌套了 ifelse 语句

[weichengwu]

写成这样了叫逻辑清晰？如果 if 代码块里面的行数超过一屏能显示的长度，逻辑是否还清晰？

[huihuilang]

建议楼主把自己的代码贴出来。。另外同事代码写得好写的不好有啥关系？自己拿自己的工资，干好自己的事情就行

[garlics]

@huihuilang 因为现在我在维护这个代码

[FallenTy]

把 tableList 里面的东西封装下，根据条件直接引用观感好很多，这也能能有人吐槽事逼？还是说你们成天就是写这种代码。不过这不算太离谱的代码，起码逻辑清晰，你同事估计就是犯懒不想优化，顺便还显得代码多

[huihuilang]

@garlics 那是够惨的。。。

[beingbin]

6 年经验×
1 年经验重复 6 年√

[tulongtou]

@weichengwu 这说明该换高分辨率的显示屏了 （手动狗头

[Shing]

莫笑前辈，总有一天你会经历一样的事情的。

[Felldeadbird]

楼主，如果你不是管理的话，那么这代码是正常的。
如果你是管理的话，push 内容时，你应该审查代码。

所以，楼主不要按工龄看代码。这个完全看个人态度的。 我有时候也会写这种代码，因为心情烦躁 + 赶工期。一般我会注明 @todo 形式，为何要这么写。

[cmdOptionKana]

@weichengwu 这个代码最开始可能很短的，后来加着加着变成这么长，如果继续加需求发现代码已经变得很不合理（比如楼主现在就刚好是这个阶段），此时可以重构。

反正感觉问题不是很大的时候就按简单快速的方法先写，可以等后面眼看越来越不合理了再改。（有时等不到那个时候项目就黄了）

[dustinth]

再看了一下楼主的补充, 自己也说了有"新"的需求, 新需求带来代码或者结构的变化是正常现象. 以我多年编程经验来看, 过早的优化比不优化其实危害更大, 不优化大不了重写一部分逻辑, 过早优化改的时候还得把所有优化过的逻辑拆开重装.

[dethan]

各位天天各种研究算法 结果遇到这种代码 还说好????

[dethan]

恕我直言 他好就好在好他****

[wxsm]

相信我，这个算是不错的。真的，十几年的码农写出来的东西不如实习生的，比比皆是。我见得多了。

[zxcslove]

原代码的写法有个大坑，就是参数里面的字符串有少许变化时，如何减少不一致的错误。而且修改时不注意发生了不一致，也很难看出来。

[busymilk]

最烦各种抽象,各种封装,除了作者本人,没有任何人能改的动.抽象调抽象,根本找不到实例化的地方,一会就绕懵逼了

[tailf]

# 我的经验


刚学会封装的工作一年的新手最喜欢多层封装，而有经验的程序员都懂：

代码是写给人看的，只是恰好能运行。

[jwenjian]

duplication is more cheap than wrong abstraction

[Biebe]

特别是那种所谓“全栈”工程师

[dc25b]

我也很奇怪为什么提一句封装这么多人急得跳脚，什么叫封装一下就看不懂了？这段代码如果是长期慢慢累积的还能理解，这要是在一个 PR 里面在我组里 review 是绝对过不了的。

[jsjjdzg]

年轻精英员工和 敲了多年看了多年代码的 2 群人的碰撞，往往是精英看不上别人的

[fangcan]

你有看下他的其他代码么

[opengps]

有些老旧代码，仅仅是因为一次配置不需要再次使用，我不懂前端代码，不确定这个是不是这一类

[ccraohng]

我会把 url, icon 等等配置写在写在一个文件里，直接组合引用。过长的静态数据最好不要放在逻辑里， 反正我是不会这样，反正也就几分钟的事。说写得好我是没想到的

[zoffy]

@wizardoz #3 这波你在第五层

[kimqcn]

可以干掉提需求的人

[Rorysky]

手动 编译器优化，你再怎么封装，最后 都是展开的

[Inside]

这波我站楼主，没有意识要做到 single point of truth 的还可以理解，但已经有这个意识了不想做、懒得做、嫌麻烦的，还是别写代码坑人了。
在我的团队里，这种问题第一次发现我会给个黄牌警告，再有下次直接红牌下场了。

[myzyq]

擦汗。忙起来还顾得了这么多。最少逻辑清楚，问题解决，后续其他人开发可以快速入场。
ps： 自己封装了，还要很多测试，不能保证功能正常，后续别人接手还要先搞清楚你的封装内容，理解也需要花时间。

不明白这个跟工作时间长短有什么关系。。。。
时间更长的人可能还会考虑，开发效率，接手难易，逻辑是否清晰。至于是否封装，不影响效率的情况下，封装不封装有区别？

[Reapper]

非 php 人员，除了$tablist 没有抽出去，感觉其他问题不大，能看懂是什么意思

[cumshot]

六年的同事还这样写，你以为他在第一层，你在第四层，其实人家在平流层，自己琢磨去吧。

[fenglangjuxu]

@tabris17 #5 你是对 php 有多大偏见啊 语言无优劣 能力有高低