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

2016-08-04 13:29:09 +08:00
 tiancaiamao

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

10601 次点击
所在节点    编程
71 条回复
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
搞个 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
我就是那个同事。
gimp
2016-08-04 19:18:21 +08:00
第二种完全没有读的欲望
loading
2016-08-04 19:22:57 +08:00
贵公司代码 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 还真有点怕那同事蹦出来说这句话呢...

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

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

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

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

© 2021 V2EX