tiancaiamao
V2EX  ›  编程

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

  •  
  •   tiancaiamao · Aug 4, 2016 · 12064 views
    This topic created in 3742 days ago, the information mentioned may be changed or developed.

    我们代码提交有比较严格的 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 replies  •  2016-08-21 18:55:28 +08:00
    yanyandenuonuo
    yanyandenuonuo
    Aug 4, 2016 via iPhone
    1
    效率高点吧 没啥好争议的
    mahone3297
    mahone3297
    Aug 4, 2016
    2
    好像。。。确实是 lz 的版本,可读性更好。。。
    申请老大仲裁啊。。。最简单。。。
    tiancaiamao
    tiancaiamao
    OP
    Aug 4, 2016
    3
    @yanyandenuonuo 局部这点写法在效率上区别,对于程序整体而言根本没有差异。
    xbb7766
    xbb7766
    Aug 4, 2016 via Android
    4
    给你同事跪了,那么多 switch … case 简直看的头发昏了要。至于性能一说更是扯, CPU 又不差这点运算能力。
    aprikyblue
    aprikyblue
    Aug 4, 2016 via Android
    5
    好像确实是。。。第二个可读性好个毛。。。性能更是扯。。。
    endlessroad1991
    endlessroad1991
    Aug 4, 2016 via iPhone
    6
    语言貌似是 go 。

    go 有 go generate ,下面那段代码可以用类似你上面那一小段来生成。
    BingoXuan
    BingoXuan
    Aug 4, 2016
    7
    可读性而言,第二个完全不存在可读性。 lz 的粗略一看就知道用来干嘛,你同事的我还要一行行看代码。
    jydeng
    jydeng
    Aug 4, 2016
    8
    第二个版本完全没有看的欲望
    dubaiyouyue
    dubaiyouyue
    Aug 4, 2016
    9
    楼主坚持住啊,一忍成千古包子后面被人随便捏!
    ChiangDi
    ChiangDi
    Aug 4, 2016❤️1
    10
    赞,公司制度这么好
    moosoome
    moosoome
    Aug 4, 2016
    11
    哈哈~要我真没耐心写那么长
    bk201
    bk201
    Aug 4, 2016 via iPhone
    12
    可以找第三个人打分,结束
    tabris17
    tabris17
    Aug 4, 2016
    13
    一般编译器会把 switch 优化成跳转表。 C 语言中如果一定要用超长 switch...case ,一般都用宏定义来优化一下。

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


    @ChiangDi 我们的代码是开源放在 github 的...被众目暌暌之下,这个问题会更让人纠结。
    9hills
    9hills
    Aug 4, 2016
    15
    @tiancaiamao 找 CodeMaster 啊,所有 code 应该都有 master ,冲突后由 Master 决定

    如果 Master 还不支持你,要么忍,要么走
    tiancaiamao
    tiancaiamao
    OP
    Aug 4, 2016
    16
    @9hills 在写代码方面做的比较民主。也就是按照目前的制度, master 或者说 boss 提代码,也是要经过两个人 review 同意了才能合并的。大概区别在于存在分歧时,说服别人同意会容易一些。

    算了我坚持现在的写法,然后争取到其它人的投票吧。
    raincious
    raincious
    Aug 4, 2016
    17
    楼主,现在 3 点,下午茶时间,你可以买个奶茶贿赂下他 LOL
    fantasyczl
    fantasyczl
    Aug 4, 2016
    18
    第二个不光是丑啊,一个函数中的代码太长,好几屏,看都不想看啊,而且容易出错
    shimanooo
    shimanooo
    Aug 4, 2016
    19
    trie: 动态地自动生成状态机
    你同事:静态手动生成状态机
    lexer generator :静态自动生成状态机
    tiancaiamao
    tiancaiamao
    OP
    Aug 4, 2016
    20
    @shimanooo 之前版本就是 lexer generator 生成代码的
    但是有些问题,维护性比较差,生成的代码是不可读的。
    性能不太好,而介于代码是生成的,不好做优化。

    现在正在做重构
    warlock
    warlock
    Aug 4, 2016
    21
    我会告诉同事:“滚犊子”
    SpicyCat
    SpicyCat
    Aug 4, 2016
    22
    怎么会出现某个人不给赞就不能 merge 的情况?难道总共就两个人 review ?
    justfly
    justfly
    Aug 4, 2016
    23
    可以考虑用 map
    herozzm
    herozzm
    Aug 4, 2016
    24
    第二种代码确实是完败
    tiancaiamao
    tiancaiamao
    OP
    Aug 4, 2016
    25
    @SpicyCat 总共就 4 5 个人...我自己不能给自己点赞,这个 PR 有点大, review 不过来,新人不适合来点赞...所以争取那个同事才会比较重要
    Orz


    @justfly map 性能更低一些,而且涉及字符回退了,就不适合了
    bigbook
    bigbook
    Aug 4, 2016
    26
    lz 高手,代码可读性好,不易出错
    同事彩笔,代码可读性差甚至都不愿看,容易出错

    直接开了丫的
    skinheadbob
    skinheadbob
    Aug 4, 2016 via iPhone
    27
    搞个 set 呗 同样的参数不用写两遍
    justfly
    justfly
    Aug 4, 2016
    28
    @tiancaiamao 嗯 没仔细看 这种前缀匹配的确实适合 Trie 就是干这个的
    kier
    kier
    Aug 4, 2016
    29
    @tiancaiamao 楼主这个代码,函数调了这么多次, 不能搞个循环吗?
    goool
    goool
    Aug 4, 2016
    30
    楼主的代码更好读,但可以搞一个 map ,像这样:

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

    然后遍历这个 map 。
    strwei
    strwei
    Aug 4, 2016
    31
    switch-case 效率高啊,性能好,楼主估计没做过大项目
    mozokov
    mozokov
    Aug 4, 2016
    32
    越简单的代码越不容易出错,反而是那种很长的代码一眼看上去都不知道 bug 在哪,第二种的写法简直是灾难
    ebony0319
    ebony0319
    Aug 4, 2016 via Android
    33
    我就是那个同事。
    gimp
    gimp
    Aug 4, 2016
    34
    第二种完全没有读的欲望
    loading
    loading
    Aug 4, 2016 via Android❤️1
    35
    贵公司代码 5 毛一行?
    Maskeney
    Maskeney
    Aug 4, 2016
    36
    楼上亮了
    Maskeney
    Maskeney
    Aug 4, 2016
    37
    好吧我说的是 33 楼
    kaizixyz
    kaizixyz
    Aug 4, 2016
    38
    我觉得性能优化比可读性的权重低。
    chmlai
    chmlai
    Aug 4, 2016
    39
    lz 的好多了, 这个地方 tire 比 map 好
    tiancaiamao
    tiancaiamao
    OP
    Aug 4, 2016
    40
    @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
    lhbc
    Aug 4, 2016❤️1
    41
    我估计你同事初中就有十万行代码的经验
    直到他学会循环
    a2ex
    a2ex
    Aug 4, 2016
    42
    一半按照你的写法,一半按照他的写法咯
    Marlon
    Marlon
    Aug 4, 2016
    43
    大赞你们的公司的开发制度。
    exch4nge
    exch4nge
    Aug 4, 2016
    44
    性能高是主要需求的话,用第二种方式好些,不好看的问题,其实像楼上有人说的一样,用宏处理一下,估计会好看很多,但我猜估计不会是这种情况。
    wupher
    wupher
    Aug 4, 2016
    45
    你们同事是傻吧……
    要么就是他觉得你傻,忽悠你呢……
    这不是一眼看过去就知道的事
    jon
    jon
    Aug 4, 2016
    46
    第二段代码能看么。。。
    ianva
    ianva
    Aug 4, 2016
    47
    估计觉得一般 lexer 都这么写,当是个定式,你写花哨了人家觉得不舒服
    PrideChung
    PrideChung
    Aug 4, 2016❤️1
    48
    这居然还有得争,菜鸟真是最喜欢拿性能来当遮羞布,然而其实连性能的量级都没搞懂
    bomb77
    bomb77
    Aug 5, 2016
    49
    是时候 py 交易了 233333333
    kooze
    kooze
    Aug 5, 2016
    50
    要是我直接写汇编干死丫的。
    troyl
    troyl
    Aug 5, 2016
    51
    我们公司也是,必须两个人以上 Approve 才能 Merge 。我一般遇到这种事情就多加 reviewer ,然后等到批准的人够了,就直接 merge 。
    jeffw
    jeffw
    Aug 5, 2016 via iPhone
    52
    要啥自行车
    Felldeadbird
    Felldeadbird
    Aug 5, 2016
    53
    我就是楼主的同事……然后晒楼主写的代码。哈哈
    bigdogbigpig
    bigdogbigpig
    Aug 5, 2016
    54
    哈哈哈哈哈,第二种直观?就是一个笑话~~~
    htfy96
    htfy96
    Aug 5, 2016 via Android
    55
    lexer 很多这种大型 switch …可能有几种原因:
    - 历史上 lexer 写的很早,对性能要求严格,教科书风格就遗传了下来
    - switch 之后要加一些奇怪的处理比较方便

    所以这取决于需求
    zacard
    zacard
    Aug 5, 2016
    56
    第二种毫无可读性
    marffin
    marffin
    Aug 5, 2016
    57
    做一下 profiling ,看看性能数据。如果不是差的特别大,让你同事去死。
    hydyy
    hydyy
    Aug 5, 2016
    58
    只看颜值~还是第一种吧!
    jason19659
    jason19659
    Aug 5, 2016
    59
    反对 switch
    102errors
    102errors
    Aug 5, 2016
    60
    至少很羡慕你们的 review 制度
    searene
    searene
    Aug 5, 2016
    61
    显然是第一种好,性能不是多个函数少个函数这么比的。

    我记得 python 里面直接把 switch case 去掉了,当然这和楼主的问题没有太大关系。
    sillyousu
    sillyousu
    Aug 5, 2016
    62
    我觉得下面直观一些。 第一种做法一眼看去不知道在做什么
    LINAICAI
    LINAICAI
    Aug 5, 2016
    63
    case 里再嵌套 case 。。。。
    确实可读性很差啊,性能就不知道了,毕竟不懂
    代码行数能当 kpi 嘛那你完败了
    tabris17
    tabris17
    Aug 5, 2016
    64
    LZ 也就只敢上 V2 吐吐槽,有本事肛他正面
    strwei
    strwei
    Aug 5, 2016 via iPhone
    65
    wzqcongcong
    wzqcongcong
    Aug 5, 2016
    66
    就目前来看,楼主的代码确实好看点。但不确定在别的场合下是否更优。 2333
    k9982874
    k9982874
    Aug 5, 2016
    67
    用 switch case 说性能好的能力就不怎么样
    ooonme
    ooonme
    Aug 5, 2016 via iPhone
    68
    @k9982874 如果编译器没有优化, switch case 确实会好,实际上编译器也是把 if else 优化成 switch ,进而采用跳跃表执行
    k9982874
    k9982874
    Aug 5, 2016 via iPhone
    69
    @ooonme 你说反了, switch case 编译时优化成类似 if else 的汇编代码。但是 switch case 会实现所有的 case 匹配语句,即使你没有显示声明 case 语句,从而用空间换时间。当 case 条件离散时会消耗相对更多空间,效能不见的更好。
    ooonme
    ooonme
    Aug 6, 2016 via iPhone
    70
    @k9982874 哦,是吗?我没有说 switch 一定好,但是大量的 if else 在现代编译器上都尽可能的优化成跳跃表,空间换时间而已 http://stackoverflow.com/questions/6805026/is-switch-faster-than-if
    mingyun
    mingyun
    Aug 21, 2016
    71
    代码量多可以加工资吗
    About   ·   Help   ·   Advertise   ·   Blog   ·   API   ·   FAQ   ·   Privacy   ·   Solana   ·   1687 Online   Highest 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 189ms · UTC 23:54 · PVG 07:54 · LAX 15:54 · JFK 18:54
    ♥ Do have faith in what you're doing.