同事非让我把代码写成这样，该怎么办？

V2EX = way to explore

V2EX 是一个关于分享和探索的地方

现在注册

已注册用户请登录

这是一个创建于 3068 天前的主题，其中的信息可能已经有所发展或是发生改变。

我们代码提交有比较严格的 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 就不给通过，他们合不了代码年底评审打分就低，一直打分低就让他让们走人，于是世界就是美好的大同社会了。

71 条回复 • 2016-08-21 18:55:28 +08:00

yanyandenuonuo

2016-08-04 13:31:49 +08:00 via iPhone

效率高点吧没啥好争议的

mahone3297

2016-08-04 13:33:53 +08:00

好像。。。确实是 lz 的版本，可读性更好。。。
申请老大仲裁啊。。。最简单。。。

tiancaiamao

2016-08-04 13:34:40 +08:00

@yanyandenuonuo 局部这点写法在效率上区别，对于程序整体而言根本没有差异。

xbb7766

2016-08-04 13:43:21 +08:00 via Android

给你同事跪了，那么多 switch … case 简直看的头发昏了要。至于性能一说更是扯， CPU 又不差这点运算能力。

aprikyblue

2016-08-04 13:54:53 +08:00 via Android

好像确实是。。。第二个可读性好个毛。。。性能更是扯。。。

endlessroad1991

2016-08-04 14:14:39 +08:00 via iPhone

语言貌似是 go 。

go 有 go generate ，下面那段代码可以用类似你上面那一小段来生成。

BingoXuan

2016-08-04 14:20:12 +08:00

可读性而言，第二个完全不存在可读性。 lz 的粗略一看就知道用来干嘛，你同事的我还要一行行看代码。

jydeng

2016-08-04 14:24:12 +08:00

第二个版本完全没有看的欲望

dubaiyouyue

2016-08-04 14:24:45 +08:00

楼主坚持住啊，一忍成千古包子后面被人随便捏！

ChiangDi

2016-08-04 14:24:47 +08:00

赞，公司制度这么好

moosoome

2016-08-04 14:29:17 +08:00

哈哈~要我真没耐心写那么长

bk201

2016-08-04 14:33:10 +08:00 via iPhone

可以找第三个人打分，结束

tabris17

2016-08-04 14:33:42 +08:00

一般编译器会把 switch 优化成跳转表。 C 语言中如果一定要用超长 switch...case ，一般都用宏定义来优化一下。

go 不清楚。不过这代码不能忍

tiancaiamao

2016-08-04 14:43:37 +08:00

@dubaiyouyue 问题就在于，他不点赞代码很难合进去。
至于被“随便捏”，理论上讲，如果单纯出于报复我以后在他提代码的时候，坚持认为他某个风格有问题不给他点赞。只是从价值观上，不能这么干。一般我 review 别人的代码比较包容，不会太介意写法。

@ChiangDi 我们的代码是开源放在 github 的...被众目暌暌之下，这个问题会更让人纠结。

9hills

2016-08-04 14:46:36 +08:00

@tiancaiamao 找 CodeMaster 啊，所有 code 应该都有 master ，冲突后由 Master 决定

如果 Master 还不支持你，要么忍，要么走

tiancaiamao

2016-08-04 14:58:51 +08:00

@9hills 在写代码方面做的比较民主。也就是按照目前的制度， master 或者说 boss 提代码，也是要经过两个人 review 同意了才能合并的。大概区别在于存在分歧时，说服别人同意会容易一些。

算了我坚持现在的写法，然后争取到其它人的投票吧。

raincious

2016-08-04 15:02:04 +08:00

楼主，现在 3 点，下午茶时间，你可以买个奶茶贿赂下他 LOL

fantasyczl

2016-08-04 16:06:41 +08:00

第二个不光是丑啊，一个函数中的代码太长，好几屏，看都不想看啊，而且容易出错

shimanooo

2016-08-04 16:13:28 +08:00

trie: 动态地自动生成状态机
你同事：静态手动生成状态机
lexer generator ：静态自动生成状态机

tiancaiamao

2016-08-04 17:12:15 +08:00

@shimanooo 之前版本就是 lexer generator 生成代码的
但是有些问题，维护性比较差，生成的代码是不可读的。
性能不太好，而介于代码是生成的，不好做优化。

现在正在做重构

warlock

2016-08-04 17:20:53 +08:00

我会告诉同事：“滚犊子”

SpicyCat

2016-08-04 17:32:46 +08:00

怎么会出现某个人不给赞就不能 merge 的情况？难道总共就两个人 review ？

justfly

2016-08-04 17:40:55 +08:00

可以考虑用 map

herozzm

2016-08-04 17:44:42 +08:00

第二种代码确实是完败

tiancaiamao

2016-08-04 17:46:38 +08:00

@SpicyCat 总共就 4 5 个人...我自己不能给自己点赞，这个 PR 有点大， review 不过来，新人不适合来点赞...所以争取那个同事才会比较重要
Orz

@justfly map 性能更低一些，而且涉及字符回退了，就不适合了

bigbook

2016-08-04 17:56:05 +08:00

lz 高手，代码可读性好，不易出错
同事彩笔，代码可读性差甚至都不愿看，容易出错

直接开了丫的

skinheadbob

2016-08-04 18:01:12 +08:00 via iPhone

搞个 set 呗同样的参数不用写两遍

justfly

2016-08-04 18:05:13 +08:00

@tiancaiamao 嗯没仔细看这种前缀匹配的确实适合 Trie 就是干这个的

kier

2016-08-04 18:06:09 +08:00

@tiancaiamao 楼主这个代码，函数调了这么多次，不能搞个循环吗？

goool

2016-08-04 18:53:02 +08:00

楼主的代码更好读，但可以搞一个 map ，像这样：

mapToken := {
'>': int('>'),
'<': int('<'),
'(': int('('),
')': int(')'),
...
}

然后遍历这个 map 。

strwei

2016-08-04 18:55:08 +08:00

switch-case 效率高啊，性能好，楼主估计没做过大项目

timwu

2016-08-04 19:10:53 +08:00

越简单的代码越不容易出错，反而是那种很长的代码一眼看上去都不知道 bug 在哪，第二种的写法简直是灾难

ebony0319

2016-08-04 19:14:25 +08:00 via Android

我就是那个同事。

gimp

2016-08-04 19:18:21 +08:00

第二种完全没有读的欲望

loading

2016-08-04 19:22:57 +08:00 via Android

贵公司代码 5 毛一行？

Maskeney

2016-08-04 19:24:55 +08:00

楼上亮了

Maskeney

2016-08-04 19:25:13 +08:00

好吧我说的是 33 楼

kaizixyz

2016-08-04 19:47:28 +08:00

我觉得性能优化比可读性的权重低。

chmlai

2016-08-04 19:58:48 +08:00

lz 的好多了, 这个地方 tire 比 map 好

tiancaiamao

2016-08-04 20:07:20 +08:00

@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 还真有点怕那同事蹦出来说这句话呢...

lhbc

2016-08-04 20:14:01 +08:00

我估计你同事初中就有十万行代码的经验
直到他学会循环

a2ex

2016-08-04 20:14:47 +08:00

一半按照你的写法，一半按照他的写法咯

Marlon

2016-08-04 20:55:03 +08:00

大赞你们的公司的开发制度。

exch4nge

2016-08-04 21:03:28 +08:00

性能高是主要需求的话，用第二种方式好些，不好看的问题，其实像楼上有人说的一样，用宏处理一下，估计会好看很多，但我猜估计不会是这种情况。

wupher

2016-08-04 22:06:39 +08:00

你们同事是傻吧……
要么就是他觉得你傻，忽悠你呢……
这不是一眼看过去就知道的事

jon

2016-08-04 22:39:18 +08:00

第二段代码能看么。。。

ianva

2016-08-04 23:06:56 +08:00

估计觉得一般 lexer 都这么写，当是个定式，你写花哨了人家觉得不舒服

PrideChung

2016-08-04 23:59:01 +08:00

这居然还有得争，菜鸟真是最喜欢拿性能来当遮羞布，然而其实连性能的量级都没搞懂

bomb77

2016-08-05 00:22:38 +08:00

是时候 py 交易了 233333333

kooze

2016-08-05 00:25:03 +08:00

要是我直接写汇编干死丫的。

troyl

2016-08-05 05:17:28 +08:00

我们公司也是，必须两个人以上 Approve 才能 Merge 。我一般遇到这种事情就多加 reviewer ，然后等到批准的人够了，就直接 merge 。

jeffw

2016-08-05 08:30:34 +08:00 via iPhone

要啥自行车

Felldeadbird

2016-08-05 08:56:50 +08:00

我就是楼主的同事……然后晒楼主写的代码。哈哈

mazyi

2016-08-05 09:10:43 +08:00

哈哈哈哈哈，第二种直观？就是一个笑话~~~

htfy96

2016-08-05 09:55:49 +08:00 via Android

lexer 很多这种大型 switch …可能有几种原因:
- 历史上 lexer 写的很早，对性能要求严格，教科书风格就遗传了下来
- switch 之后要加一些奇怪的处理比较方便

所以这取决于需求

zacard

2016-08-05 10:00:09 +08:00

第二种毫无可读性

marffin

2016-08-05 10:38:52 +08:00

做一下 profiling ，看看性能数据。如果不是差的特别大，让你同事去死。

hydyy

2016-08-05 11:42:26 +08:00

只看颜值~还是第一种吧！

jason19659

2016-08-05 11:48:38 +08:00

反对 switch

102errors

2016-08-05 11:56:27 +08:00

至少很羡慕你们的 review 制度

searene

2016-08-05 12:21:13 +08:00

显然是第一种好，性能不是多个函数少个函数这么比的。

我记得 python 里面直接把 switch case 去掉了，当然这和楼主的问题没有太大关系。

sillyousu

2016-08-05 13:19:43 +08:00

我觉得下面直观一些。第一种做法一眼看去不知道在做什么

LINAICAI

2016-08-05 13:26:21 +08:00

case 里再嵌套 case 。。。。
确实可读性很差啊，性能就不知道了，毕竟不懂
代码行数能当 kpi 嘛那你完败了

tabris17

2016-08-05 13:30:31 +08:00

LZ 也就只敢上 V2 吐吐槽，有本事肛他正面

strwei

2016-08-05 13:38:30 +08:00 via iPhone

你们看看这个 @lincanbin 的 https://github.com/lincanbinanbin/Carbon-ForForum

wzqcongcong

2016-08-05 14:33:19 +08:00

就目前来看，楼主的代码确实好看点。但不确定在别的场合下是否更优。 2333

k9982874

2016-08-05 14:46:29 +08:00

用 switch case 说性能好的能力就不怎么样

ooonme

2016-08-05 18:49:24 +08:00 via iPhone

@k9982874 如果编译器没有优化， switch case 确实会好，实际上编译器也是把 if else 优化成 switch ，进而采用跳跃表执行

k9982874

2016-08-05 20:29:21 +08:00 via iPhone

@ooonme 你说反了， switch case 编译时优化成类似 if else 的汇编代码。但是 switch case 会实现所有的 case 匹配语句，即使你没有显示声明 case 语句，从而用空间换时间。当 case 条件离散时会消耗相对更多空间，效能不见的更好。

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

mingyun

2016-08-21 18:55:28 +08:00

代码量多可以加工资吗