我们代码提交有比较严格的 review 机制,必须得到两个人以上点赞了,才能够 merge 。
写 lexer 的时候,提交的一段代码,用了个 trie 结构打了张表,遍历得到相应的 token 。
// search a trie to scan a token ch := ch0 node := &ruleTable for { if node.childs[ch] == nil || s.r.eof() { break } node = node.childs[ch] s.r.inc() ch = s.r.peek() } initTokenByte('>', int('>')) initTokenByte('<', int('<')) initTokenByte('(', int('(')) initTokenByte(')', int(')')) initTokenByte(';', int(';')) initTokenByte(',', int(',')) initTokenByte('&', int('&')) initTokenByte('%', int('%')) initTokenByte(':', int(':')) initTokenByte('|', int('|')) initTokenByte('!', int('!')) initTokenByte('^', int('^')) initTokenByte('~', int('~')) initTokenByte('\\', int('\\')) initTokenByte('?', placeholder) initTokenByte('=', eq) initTokenString("||", oror) initTokenString("&&", andand) initTokenString("&^", andnot) initTokenString(":=", assignmentEq) initTokenString("<=>", nulleq) initTokenString(">=", ge) initTokenString("<=", le) initTokenString("!=", neq) initTokenString("<>", neqSynonym) initTokenString("<<", lsh) initTokenString(">>", rsh)
有个同事非要把代码改成这样子,用非常长的 switch-case ,代码丑到暴:
switch ch0 { case '|': s.r.inc() if s.r.peek() == '|' { s.r.inc() return oror } return '|' case '&': s.r.inc() switch s.r.peek() { case '&': s.r.inc() return andand case '^': s.r.inc() return andnot } return '|' case '<': s.r.inc() ch1 := s.r.inc() switch ch1 { case '=': s.r.inc() if s.r.peek() == '>' { s.r.inc() return '>' } return '=' } return '<' case '!': s.r.inc() if s.r.peek() == '=' { s.r.inc() return neq } return '!' case '@': return s.startWithAt() case '/': return s.startWithSlash() case '-': return s.startWithDash() case '#': s.r.incAsLongAs(func(ch byte) bool { return ch != '\n' }) return s.scan() } ...以下省略几百行的 switch-case ...
争执在于,他说这样写可读性好,性能高。他认为代码长一点不重要。他认为这样写的代码最直观。 switch-case 里面长一点的缩进提出到函数里就行了。
我认为写成这样可读性一点都不好。性能高的那一点点根本不值得把代码写这么丑。代码长了直接影响阅读。提到函数里不会让总的代码量减少,不会被重用的代码提成函数没太多价值。
现在我没法说服他,他没法说服我,但是他不给赞同就没法合进去。我又不想为了得到赞同把自己不喜欢的代码提交进去。 遇到这种情况我该怎么办?
最后补一个笑话:据说写 C++的人分很多派系,有的人喜欢 boost 。有的人不喜欢 boost ,对付异类很简单,用了 boost 的代码 review 就不给通过,他们合不了代码年底评审打分就低,一直打分低就让他让们走人,于是世界就是美好的大同社会了。
![]() | 1 yanyandenuonuo 2016-08-04 13:31:49 +08:00 via iPhone 效率高点吧 没啥好争议的 |
![]() | 2 mahone3297 2016-08-04 13:33:53 +08:00 好像。。。确实是 lz 的版本,可读性更好。。。 申请老大仲裁啊。。。最简单。。。 |
![]() | 3 tiancaiamao OP @yanyandenuonuo 局部这点写法在效率上区别,对于程序整体而言根本没有差异。 |
![]() | 4 xbb7766 2016-08-04 13:43:21 +08:00 via Android 给你同事跪了,那么多 switch … case 简直看的头发昏了要。至于性能一说更是扯, CPU 又不差这点运算能力。 |
5 aprikyblue 2016-08-04 13:54:53 +08:00 via Android 好像确实是。。。第二个可读性好个毛。。。性能更是扯。。。 |
6 endlessroad1991 2016-08-04 14:14:39 +08:00 via iPhone 语言貌似是 go 。 go 有 go generate ,下面那段代码可以用类似你上面那一小段来生成。 |
![]() | 7 BingoXuan 2016-08-04 14:20:12 +08:00 可读性而言,第二个完全不存在可读性。 lz 的粗略一看就知道用来干嘛,你同事的我还要一行行看代码。 |
![]() | 8 jydeng 2016-08-04 14:24:12 +08:00 第二个版本完全没有看的欲望 |
9 dubaiyouyue 2016-08-04 14:24:45 +08:00 楼主坚持住啊,一忍成千古包子后面被人随便捏! |
![]() | 10 ChiangDi 2016-08-04 14:24:47 +08:00 ![]() 赞,公司制度这么好 |
![]() | 11 moosoome 2016-08-04 14:29:17 +08:00 哈哈~要我真没耐心写那么长 |
![]() | 12 bk201 2016-08-04 14:33:10 +08:00 via iPhone 可以找第三个人打分,结束 |
![]() | 13 tabris17 2016-08-04 14:33:42 +08:00 一般编译器会把 switch 优化成跳转表。 C 语言中如果一定要用超长 switch...case ,一般都用宏定义来优化一下。 go 不清楚。不过这代码不能忍 |
![]() | 14 tiancaiamao OP @dubaiyouyue 问题就在于,他不点赞代码很难合进去。 至于被“随便捏”,理论上讲,如果单纯出于报复我以后在他提代码的时候,坚持认为他某个风格有问题不给他点赞。只是从价值观上,不能这么干。一般我 review 别人的代码比较包容,不会太介意写法。 @ChiangDi 我们的代码是开源放在 github 的...被众目暌暌之下,这个问题会更让人纠结。 |
![]() | 15 9hills 2016-08-04 14:46:36 +08:00 |
![]() | 16 tiancaiamao OP @9hills 在写代码方面做的比较民主。也就是按照目前的制度, master 或者说 boss 提代码,也是要经过两个人 review 同意了才能合并的。大概区别在于存在分歧时,说服别人同意会容易一些。 算了我坚持现在的写法,然后争取到其它人的投票吧。 |
17 raincious 2016-08-04 15:02:04 +08:00 楼主,现在 3 点,下午茶时间,你可以买个奶茶贿赂下他 LOL |
![]() | 18 fantasyczl 2016-08-04 16:06:41 +08:00 第二个不光是丑啊,一个函数中的代码太长,好几屏,看都不想看啊,而且容易出错 |
19 shimanooo 2016-08-04 16:13:28 +08:00 trie: 动态地自动生成状态机 你同事:静态手动生成状态机 lexer generator :静态自动生成状态机 |
![]() | 20 tiancaiamao OP |
![]() | 21 warlock 2016-08-04 17:20:53 +08:00 我会告诉同事:“滚犊子” |
22 SpicyCat 2016-08-04 17:32:46 +08:00 怎么会出现某个人不给赞就不能 merge 的情况?难道总共就两个人 review ? |
![]() | 23 justfly 2016-08-04 17:40:55 +08:00 可以考虑用 map |
![]() | 24 herozzm 2016-08-04 17:44:42 +08:00 第二种代码确实是完败 |
![]() | 25 tiancaiamao OP |
26 bigbook 2016-08-04 17:56:05 +08:00 lz 高手,代码可读性好,不易出错 同事彩笔,代码可读性差甚至都不愿看,容易出错 直接开了丫的 |
27 skinheadbob 2016-08-04 18:01:12 +08:00 via iPhone 搞个 set 呗 同样的参数不用写两遍 |
![]() | 28 justfly 2016-08-04 18:05:13 +08:00 @tiancaiamao 嗯 没仔细看 这种前缀匹配的确实适合 Trie 就是干这个的 |
29 kier 2016-08-04 18:06:09 +08:00 @tiancaiamao 楼主这个代码,函数调了这么多次, 不能搞个循环吗? |
![]() | 30 goool 2016-08-04 18:53:02 +08:00 楼主的代码更好读,但可以搞一个 map ,像这样: mapToken := { '>': int('>'), '<': int('<'), '(': int('('), ')': int(')'), ... } 然后遍历这个 map 。 |
![]() | 31 strwei 2016-08-04 18:55:08 +08:00 switch-case 效率高啊,性能好,楼主估计没做过大项目 |
![]() | 32 timwu 2016-08-04 19:10:53 +08:00 越简单的代码越不容易出错,反而是那种很长的代码一眼看上去都不知道 bug 在哪,第二种的写法简直是灾难 |
![]() | 33 ebony0319 2016-08-04 19:14:25 +08:00 via Android 我就是那个同事。 |
![]() | 34 gimp 2016-08-04 19:18:21 +08:00 第二种完全没有读的欲望 |
![]() | 35 loading 2016-08-04 19:22:57 +08:00 via Android ![]() 贵公司代码 5 毛一行? |
![]() | 36 Maskeney 2016-08-04 19:24:55 +08:00 楼上亮了 |
![]() | 37 Maskeney 2016-08-04 19:25:13 +08:00 好吧我说的是 33 楼 |
![]() | 38 kaizixyz 2016-08-04 19:47:28 +08:00 我觉得性能优化比可读性的权重低。 |
![]() | 39 chmlai 2016-08-04 19:58:48 +08:00 lz 的好多了, 这个地方 tire 比 map 好 |
![]() | 40 tiancaiamao OP @bigbook 同事不菜的...菜的也不会有机会 review 代码啊。 事实上,确实可以找到很多例子,都是 switch-cash 风格的...也没有特别乱... https://github.com/golang/go/blob/master/src/go/scanner/scanner.go#L633 https://github.com/cockroachdb/cockroach/blob/master/sql/parser/scan.go#L219 https://github.com/influxdata/influxdb/blob/master/influxql/scanner.go#L47 (注: influxsq 是一个阉割版的 sql ,作为参考依据其实并不太好) 只是我真的特别不想这么写 @ebony0319 还真有点怕那同事蹦出来说这句话呢... |
![]() | 41 lhbc 2016-08-04 20:14:01 +08:00 ![]() 我估计你同事初中就有十万行代码的经验 直到他学会循环 |
42 a2ex 2016-08-04 20:14:47 +08:00 一半按照你的写法,一半按照他的写法咯 |
![]() | 43 Marlon 2016-08-04 20:55:03 +08:00 大赞你们的公司的开发制度。 |
![]() | 44 exch4nge 2016-08-04 21:03:28 +08:00 性能高是主要需求的话,用第二种方式好些,不好看的问题,其实像楼上有人说的一样,用宏处理一下,估计会好看很多,但我猜估计不会是这种情况。 |
![]() | 45 wupher 2016-08-04 22:06:39 +08:00 你们同事是傻吧…… 要么就是他觉得你傻,忽悠你呢…… 这不是一眼看过去就知道的事 |
46 jon 2016-08-04 22:39:18 +08:00 第二段代码能看么。。。 |
![]() | 47 ianva 2016-08-04 23:06:56 +08:00 估计觉得一般 lexer 都这么写,当是个定式,你写花哨了人家觉得不舒服 |
![]() | 48 PrideChung 2016-08-04 23:59:01 +08:00 ![]() 这居然还有得争,菜鸟真是最喜欢拿性能来当遮羞布,然而其实连性能的量级都没搞懂 |
![]() | 49 bomb77 2016-08-05 00:22:38 +08:00 是时候 py 交易了 233333333 |
![]() | 50 kooze 2016-08-05 00:25:03 +08:00 要是我直接写汇编干死丫的。 |
![]() | 51 troyl 2016-08-05 05:17:28 +08:00 我们公司也是,必须两个人以上 Approve 才能 Merge 。我一般遇到这种事情就多加 reviewer ,然后等到批准的人够了,就直接 merge 。 |
52 jeffw 2016-08-05 08:30:34 +08:00 via iPhone 要啥自行车 |
![]() | 53 Felldeadbird 2016-08-05 08:56:50 +08:00 我就是楼主的同事……然后晒楼主写的代码。哈哈 |
![]() | 54 mazyi PRO 哈哈哈哈哈,第二种直观?就是一个笑话~~~ |
![]() | 55 htfy96 2016-08-05 09:55:49 +08:00 via Android lexer 很多这种大型 switch …可能有几种原因: - 历史上 lexer 写的很早,对性能要求严格,教科书风格就遗传了下来 - switch 之后要加一些奇怪的处理比较方便 所以这取决于需求 |
56 zacard 2016-08-05 10:00:09 +08:00 第二种毫无可读性 |
57 marffin 2016-08-05 10:38:52 +08:00 做一下 profiling ,看看性能数据。如果不是差的特别大,让你同事去死。 |
![]() | 58 hydyy 2016-08-05 11:42:26 +08:00 只看颜值~还是第一种吧! |
![]() | 59 jason19659 2016-08-05 11:48:38 +08:00 反对 switch |
60 102errors 2016-08-05 11:56:27 +08:00 至少很羡慕你们的 review 制度 |
![]() | 61 searene 2016-08-05 12:21:13 +08:00 显然是第一种好,性能不是多个函数少个函数这么比的。 我记得 python 里面直接把 switch case 去掉了,当然这和楼主的问题没有太大关系。 |
![]() | 62 sillyousu 2016-08-05 13:19:43 +08:00 我觉得下面直观一些。 第一种做法一眼看去不知道在做什么 |
![]() | 63 LINAICAI 2016-08-05 13:26:21 +08:00 case 里再嵌套 case 。。。。 确实可读性很差啊,性能就不知道了,毕竟不懂 代码行数能当 kpi 嘛那你完败了 |
![]() | 64 tabris17 2016-08-05 13:30:31 +08:00 LZ 也就只敢上 V2 吐吐槽,有本事肛他正面 |
![]() | 65 strwei 2016-08-05 13:38:30 +08:00 via iPhone |
66 wzqcongcong 2016-08-05 14:33:19 +08:00 就目前来看,楼主的代码确实好看点。但不确定在别的场合下是否更优。 2333 |
![]() | 67 k9982874 2016-08-05 14:46:29 +08:00 用 switch case 说性能好的能力就不怎么样 |
![]() | 68 ooonme 2016-08-05 18:49:24 +08:00 via iPhone @k9982874 如果编译器没有优化, switch case 确实会好,实际上编译器也是把 if else 优化成 switch ,进而采用跳跃表执行 |
![]() | 69 k9982874 2016-08-05 20:29:21 +08:00 via iPhone @ooonme 你说反了, switch case 编译时优化成类似 if else 的汇编代码。但是 switch case 会实现所有的 case 匹配语句,即使你没有显示声明 case 语句,从而用空间换时间。当 case 条件离散时会消耗相对更多空间,效能不见的更好。 |
![]() | 70 ooonme 2016-08-06 00:15:12 +08:00 via iPhone @k9982874 哦,是吗?我没有说 switch 一定好,但是大量的 if else 在现代编译器上都尽可能的优化成跳跃表,空间换时间而已 http://stackoverflow.com/questions/6805026/is-switch-faster-than-if |
71 mingyun 2016-08-21 18:55:28 +08:00 代码量多可以加工资吗 |