tiancaiamao
V2EX  ›  编程

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

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

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

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


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

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

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

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


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

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

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

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

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

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