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

2020-09-27 09:58:01 +08:00
 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',
                    ]
                ];
            }
        }
29637 次点击
所在节点    程序员
298 条回复
shenjinpeng
2020-09-27 15:03:09 +08:00
首先你不说这段代码曾经改过几次, 或者是某个深夜收到需求加班赶出来的 . 评价代码好坏离不开实际的业务需求 . 把所有选项拆出来再组合的前提是格式固定, 没有定制化需求的前提 . 最后, 说实话, 楼主封装后的代码效率低下了很多倍, 内存占用多了 n 个数量级 . 封不封装取决于后面的需求还会不会变更, 是否还会增加 ifelse , 团队代码统一风格, 项目的大小以及投入的时间成本 .
no1xsyzy
2020-09-27 15:04:50 +08:00
不是挺符合函数式的么（
$tab[] = 这种语法是指 append 或者 prepend ？
所谓封装应该是写个 make_tablist 函数，根据表项 list 和通过 context 或者 thread cell 传递的 theme 来构造 tablist
lovecy
2020-09-27 15:13:54 +08:00
楼主优化过后的代码简洁了，但是对逻辑要求更高了，至少新人一眼能看懂原代码，但要花点心思读你的代码。
#181 楼说的挺好
mrkelly
2020-09-27 15:14:58 +08:00
刚入行半年的时候，看到前辈的代码，也很困惑，主动找了领导聊。
当时领导说：「有时候，越简单、越易读越好，适合团队协作」。

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

经验是：

只要架构稳定，局部代码，永远都会迭代，可读性>魔幻性，先快写；
但是，整体架构，必须要想得长远。
flicking2015
2020-09-27 15:15:32 +08:00
正常，我也工作快 6 年了，和我最近写的前端代码差不多（ ps：搞后端的）
AJQA
2020-09-27 15:29:43 +08:00
工作 3 年后从自己做外包开始 重新审视什么是 code review：
一群人在投影机前 建议代码要怎么写 一行一行一块块下来 我的总结就这：没产生实际价值 宝贵的生命被浪费了
lucifineil
2020-09-27 15:30:27 +08:00
清晰易懂，真封装了，六年后能不能看懂都是个问题
tanjian
2020-09-27 15:46:36 +08:00
没毛病，用的少，我也懒得封装，用的多的我才想去封装一下！
soulmt
2020-09-27 15:47:05 +08:00
@mrkelly 没有远见的可读性就是对可读性最大的不尊重，试想这段代码目前只有 4 块，将来业务逻辑复杂，变成 10 块，或者场景变成 20 个，甚至更多，请问，还有可读性可言么？ 屏幕竖起来滑半天可能才能窥探这个配置信息的逻辑是什么
Nostalgiaaaa
2020-09-27 15:47:33 +08:00
$config['theme'] == 1
这个 1 好歹写个常量吧，真较真槽点还是有的。
securityCoding
2020-09-27 15:51:51 +08:00
可读性排第一,这段代码并没有什么问题
soulmt
2020-09-27 15:52:16 +08:00
@jackrelative 等你维护了老代码就知道了， 希望你维护了之后，或者遇到类似代码跳脚之前，赶紧离职
wangyzj
2020-09-27 15:53:33 +08:00
其实这是一个权衡的问题
相比这么写
过度封装更影响阅读
所以权衡好就可以了，毕竟有些业务真的复用性很低
Nostalgiaaaa
2020-09-27 15:58:08 +08:00
或者 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
2020-09-27 16:01:04 +08:00
@afx 害，是我想太多。
easymbol
2020-09-27 16:10:47 +08:00
工作嘛，实现为主，真正有意思的代码是开源的
bk201
2020-09-27 16:22:08 +08:00
提前做些无关紧要的事情没啥意义。
lsj8924
2020-09-27 16:23:39 +08:00
没有对比就没有伤害，请写出你自己优化后的代码，让大家看看到底哪个更好。
javapythongo
2020-09-27 16:35:31 +08:00
我觉得优化是有一个过程的，如果最开始业务就挺简单，就简单写，当业务复杂起来时，再对代码进行优化
sunziren
2020-09-27 16:38:00 +08:00
新的一页，新的开始

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

https://www.v2ex.com/t/710808

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

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

© 2021 V2EX