我们代码提交有比较严格的 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 1
赞,公司制度这么好
|
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 1
贵公司代码 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 1
我估计你同事初中就有十万行代码的经验
直到他学会循环 |
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 1
这居然还有得争,菜鸟真是最喜欢拿性能来当遮羞布,然而其实连性能的量级都没搞懂
|
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 2016-08-05 09:10:43 +08:00
哈哈哈哈哈,第二种直观?就是一个笑话~~~
|
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
代码量多可以加工资吗
|