同事非让我把代码写成这样,该怎么办? - V2EX
V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
tiancaiamao
V2EX    编程

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

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

    我们代码提交有比较严格的 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  
    PRO
       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     Solana     977 人在线   最高记录 6679       Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 31ms UTC 23:11 PVG 07:11 LAX 16:11 JFK 19:11
    Do have faith in what you're doing.
    ubao snddm index pchome yahoo rakuten mypaper meadowduck bidyahoo youbao zxmzxm asda bnvcg cvbfg dfscv mmhjk xxddc yybgb zznbn ccubao uaitu acv GXCV ET GDG YH FG BCVB FJFH CBRE CBC GDG ET54 WRWR RWER WREW WRWER RWER SDG EW SF DSFSF fbbs ubao fhd dfg ewr dg df ewwr ewwr et ruyut utut dfg fgd gdfgt etg dfgt dfgd ert4 gd fgg wr 235 wer3 we vsdf sdf gdf ert xcv sdf rwer hfd dfg cvb rwf afb dfh jgh bmn lgh rty gfds cxv xcv xcs vdas fdf fgd cv sdf tert sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf sdf shasha9178 shasha9178 shasha9178 shasha9178 shasha9178 liflif2 liflif2 liflif2 liflif2 liflif2 liblib3 liblib3 liblib3 liblib3 liblib3 zhazha444 zhazha444 zhazha444 zhazha444 zhazha444 dende5 dende denden denden2 denden21 fenfen9 fenf619 fen619 fenfe9 fe619 sdf sdf sdf sdf sdf zhazh90 zhazh0 zhaa50 zha90 zh590 zho zhoz zhozh zhozho zhozho2 lislis lls95 lili95 lils5 liss9 sdf0ty987 sdft876 sdft9876 sdf09876 sd0t9876 sdf0ty98 sdf0976 sdf0ty986 sdf0ty96 sdf0t76 sdf0876 df0ty98 sf0t876 sd0ty76 sdy76 sdf76 sdf0t76 sdf0ty9 sdf0ty98 sdf0ty987 sdf0ty98 sdf6676 sdf876 sd876 sd876 sdf6 sdf6 sdf9876 sdf0t sdf06 sdf0ty9776 sdf0ty9776 sdf0ty76 sdf8876 sdf0t sd6 sdf06 s688876 sd688 sdf86