V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
tiancaiamao
V2EX  ›  编程

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

  •  
  •   tiancaiamao · 2016-08-04 13:29:09 +08:00 · 9632 次点击
    这是一个创建于 2196 天前的主题,其中的信息可能已经有所发展或是发生改变。

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

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

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


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

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

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

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


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

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

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

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

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

    我记得 python 里面直接把 switch case 去掉了,当然这和楼主的问题没有太大关系。
    sillyousu
        62
    sillyousu  
       2016-08-05 13:19:43 +08:00
    我觉得下面直观一些。 第一种做法一眼看去不知道在做什么
    LINAICAI
        63
    LINAICAI  
       2016-08-05 13:26:21 +08:00
    case 里再嵌套 case 。。。。
    确实可读性很差啊,性能就不知道了,毕竟不懂
    代码行数能当 kpi 嘛那你完败了
    tabris17
        64
    tabris17  
       2016-08-05 13:30:31 +08:00
    LZ 也就只敢上 V2 吐吐槽,有本事肛他正面
    strwei
        65
    strwei  
       2016-08-05 13:38:30 +08:00 via iPhone
    wzqcongcong
        66
    wzqcongcong  
       2016-08-05 14:33:19 +08:00
    就目前来看,楼主的代码确实好看点。但不确定在别的场合下是否更优。 2333
    k9982874
        67
    k9982874  
       2016-08-05 14:46:29 +08:00
    用 switch case 说性能好的能力就不怎么样
    ooonme
        68
    ooonme  
       2016-08-05 18:49:24 +08:00 via iPhone
    @k9982874 如果编译器没有优化, switch case 确实会好,实际上编译器也是把 if else 优化成 switch ,进而采用跳跃表执行
    k9982874
        69
    k9982874  
       2016-08-05 20:29:21 +08:00 via iPhone
    @ooonme 你说反了, switch case 编译时优化成类似 if else 的汇编代码。但是 switch case 会实现所有的 case 匹配语句,即使你没有显示声明 case 语句,从而用空间换时间。当 case 条件离散时会消耗相对更多空间,效能不见的更好。
    ooonme
        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
    mingyun
        71
    mingyun  
       2016-08-21 18:55:28 +08:00
    代码量多可以加工资吗
    关于   ·   帮助文档   ·   API   ·   FAQ   ·   我们的愿景   ·   广告投放   ·   感谢   ·   实用小工具   ·   3251 人在线   最高记录 5497   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 43ms · UTC 04:29 · PVG 12:29 · LAX 21:29 · JFK 00:29
    Developed with CodeLauncher
    ♥ Do have faith in what you're doing.