• 请不要在回答技术问题时复制粘贴 AI 生成的内容
hikarugo
V2EX  ›  程序员

新公司,代码审查的时候 leader 修改了一些我个人觉得真的没必要的地方

  •  1
     
  •   hikarugo · Jun 29, 2018 · 17661 views
    This topic created in 2906 days ago, the information mentioned may be changed or developed.

    for k, v in pairs(icons) do (一些调用) end

    for 里几行代码,然后把 v 改成了 item。。。。改成 icon 我也服气一点啊。。。(这里的 kv 真的是习惯了,如果是 python 的话我就会写 for icon in icons:)

    一个初始化图片 uri 的方法名:initSkinImage => initSkin.......

    随机显示一些按钮的文本方法名:randSelectLabel => randomOptions

    一些变量名: GROUP_INDEX = "group_index" => GROUP_INDEX = "gi" USER_STATUS_DATA = ” user_status_data" => USER_STATUS_DATA = "u_sd"

    一个调用我是拆开的 a.do1() a.do2() => a.do1().do2()

    更新用户状态方法名 updateUserRes => updateUserResponse

    最不能忍的是这个。 image.setVisible(true) => image.show()

    show 的内部实现就是调用 setVisible,两个方法都是框架自带的 照这种改法,现有代码里的所有 setVisible 调用处都该改了

    其他有些地方是命名是修改得好的,比如 title => titles, enterStartView => enterReadyView

    但是像上面那些的真的有点不太能接受。。。唉

    Supplement 1  ·  Jun 29, 2018
    补充一句,好多人说风格问题,可是我真的除了那个 res 的缩写之外(这个是我的错,就算缩写也应该是 resp ),其他的修改完全看不到有任何属于能被风格所定义的修改范畴啊! 又不是什么驼峰大括号 abc 这样的。。。希望你们能先好好看看修改的部分可以吗。。。
    Supplement 2  ·  Jun 29, 2018
    谢谢大家的回复,可能是一周内我这个小模块需求就添加了三次(注意是添加),本身自己就算是挺用心的在重构了,所以这些小问题的修改确实影响到了一点自己的情绪。大家说的话我也都看了,我也会选择适合自己的解决方法,谢谢大家
    Supplement 3  ·  Jun 29, 2018
    最后一条附言
    首先反驳一下那些说 show 比 setVisible 好的,考虑一下要通过某个数据状态来更新显示显示状态的情况
    setVisible(isFin)还是 if isFin then show() else hide() end
    就知道为什么我习惯写 setVisble 了。没有说 show 不好,只不过 setVisble 也没有不好,所有改动没必要

    最后:leader 很好,很 nice,对事不对人,我个人观点依然觉得那几处(除了 res )之外依旧是无关紧要,我也认为 review 是很好的,但是我更偏向看到一些代码性的修改:比如修改了某个算法,提高了性能,或者修复了某些隐藏 bug 之类的。而不是在已有代码就存在我被修改的调用(刚搜了下 setVisible: 204 matches across 56 files)和一些冗余的使用 temp 而不是用 a,b = b, a 交换的情况下。这样显得修改建议并没有那么有说服力,反而有点画蛇添足?

    最后的最后,大家周末愉快~ 明天约妹子看电影了,紧脏。。。。
    131 replies    2018-07-05 22:33:45 +08:00
    1  2  
    jorneyr
        101
    jorneyr  
       Jun 29, 2018
    随机显示一些按钮的文本方法名:randSelectLabel => randomOptions

    一个是单数,一个是复数,最后随机显示的是单数还是复数呢?如果不确定是单数还是复数大家就不知道那个好,但至少有一个是有问题的。
    huijiewei
        102
    huijiewei  
       Jun 29, 2018 via iPhone
    下周不去了

    咱不惯着这种 leader

    就这样子
    cnTangLang
        103
    cnTangLang  
       Jun 29, 2018
    对你来说可能没必要
    对他未必
    不要拿自己的所知和思维来衡量别人的所知和思维,这是职场基本规则
    yangqi
        104
    yangqi  
       Jun 29, 2018
    很正常,没有对错,但是 leader 要负责整个组的代码一致性,不然每个人都自己的风格,之后维护起来很麻烦的。所以单个看每个修改都可以说没必要,但是从全局来看就是有必要。
    BruceLi
        105
    BruceLi  
       Jun 29, 2018
    和 leader 深入讨论:1 )如果你对但是 leader 不接受,那么以后日子也不会太舒服,趁早想办法吧;如果 leader 接受了,对你有加分。2 )如果 leader 对,你学到了改就是了。总之多沟通,不要憋着,自己内心不爽,对大家都没好处。
    a7a2
        106
    a7a2  
       Jun 29, 2018
    无看完 ,第一个 for 的,你是对的。k,v 足够简洁干练一看就知道 key,value,绝对比 item 实际。
    coolcfan
        107
    coolcfan  
       Jun 29, 2018
    你得这么想,如果过很久之后换个人来读你的代码,他能不能读懂。

    我看惯了我司的代码风格,读别家代码的时候看到满屏的缩写就头疼。
    M003
        108
    M003  
       Jun 30, 2018   ❤️ 1
    有强制规定还是比较好的,毕竟大家都统一,接手也方便,

    你们试着接手 变量有 $aa,$a

    我找谁说理去?
    iaigcfun
        109
    iaigcfun  
       Jun 30, 2018
    leader 的水平确实比你高很多,素养问题啊。Option 就是 Option,不是什么 selectedLabel
    fuermosi777
        110
    fuermosi777  
       Jun 30, 2018
    你写代码要考虑的是后来人看你代码的感受。
    ToT
        111
    ToT  
       Jun 30, 2018
    改了以后,就照着这个命名方式来写。老板希望团队统一命名 和 习惯,没有必要钻这个牛角尖。
    POPOEVER
        112
    POPOEVER  
       Jun 30, 2018
    新公司没先给你看 Best Practice 或者开发规范么?
    gtanyin
        113
    gtanyin  
       Jun 30, 2018
    改就改呗,哪有这么多*事?还来 v 站发个帖。。。。。。。。
    lrh3321
        114
    lrh3321  
       Jun 30, 2018 via Android
    统一代码风格
    DavidNineRoc
        115
    DavidNineRoc  
       Jun 30, 2018
    说实话响应我习惯用 res 了
    HangoX
        116
    HangoX  
       Jun 30, 2018 via Android
    leader 是让你写团队风格的代码,对团队有很大的好处。
    zjsxwc
        117
    zjsxwc  
       Jun 30, 2018 via iPhone
    “最不能忍的是这个。image.setVisible(true) => image.show()

    show 的内部实现就是调用 setVisible,两个方法都是框架自带的 照这种改法,现有代码里的所有 setVisible 调用处都该改了”

    兄弟,你没碰到过以后如果 show 的时候要多加点业务的场景吗?比如不单单是图片直接展示,你还要家点特效这类的,直接 setvisible 就写死了以后不好改。
    wangxn
        118
    wangxn  
       Jun 30, 2018 via Android
    游戏行业有这种细致要求的老大真少见。
    还有,visible 是个形容词,setVisible 极为奇怪。
    hadixlin
        119
    hadixlin  
       Jun 30, 2018
    这种事儿是想法的的碰撞,领导在给你表达想法的机会,代码审查其实是程序员之间沟通想法的通道.

    光在这里发牢骚,不如去沟通.
    zcjfesky
        120
    zcjfesky  
       Jun 30, 2018 via Android
    现在当组长真是… 不审吧,组员可能会上 v2 吐槽你不干活;审得细吧,组员还是会上 v2 吐槽…

    编码规范很重要,你写的代码不是只给你自己看的。如果公司没有编码规范纯靠人工经验,那你得督促公司推进规范文件
    leekafai
        121
    leekafai  
       Jun 30, 2018 via Android
    强迫症,不行就分
    bookit
        122
    bookit  
       Jun 30, 2018
    和此人讨论一下,看看有没有代码规范,有就学习一下。

    谈不拢就走人呗
    someonedeng
        123
    someonedeng  
       Jun 30, 2018
    除了 response 那个,其他改动没什么必要。。
    jmk92
        124
    jmk92  
       Jun 30, 2018 via iPhone
    我也遇到过这样的 lender,但是影响更深的是也有一个你这样的同事,处女座是不是?
    Coioidea
        125
    Coioidea  
       Jun 30, 2018
    首先你们公司有 code conduct 吗?你或你的 leader 谁遵守了?
    然后公司其他的相关代码你都读过吗?有习惯用法或注释都了解吗?
    一个好的团队应该是有标准化的管理的,无规矩不成方圆
    wizardoz
        126
    wizardoz  
       Jun 30, 2018
    我想说我看过的 response 缩写一般是 rsp,res 一般理解为 resource
    scofieldpeng
        127
    scofieldpeng  
       Jun 30, 2018
    非常同意 5 楼的做法,代码规范这个东西,每个公司都有不同的规范,作为新人,大忌就是把之前公司和个人习惯带过来。举个例子,for 循环里面可能你在前一家公司用的是 k,v,现在的公司用的是 i,v,你说哪个好?其实都一样,但是如果你把你前面公司的习惯带过来,还是之前说的,10 个人的团队如果和你一样,连一个简单的 for 循环都有 11 种不同的写法,假设有 1 个函数你们 11 个人都维护,你会发现这个函数里面的代码简直就是无法直视,每个人都会说自己的是规范,比起这个,为什么不用 leader 强制的规范,是,可能你说的这些觉得他很不“规范”,但是记住,能让团队高效协作的就是好规范。

    其实规范这东西和人类的语言很相似,全世界不说,就中国就不知道多少种方言,如果没有普通话这个官方指定语言,你觉得你能和其他地方的人流畅沟通,别逗了?那你能说普通话就是最规范最牛逼的么?
    ninestep
        128
    ninestep  
       Jun 30, 2018
    你只是改了,你没遇到过你写了一天,看了三分钟给你全删了让你重写的呢,询问原因他的回复是你觉得能用?废话我可是都做过单元测试的,关键后来我用 git 恢复了他说写的不错~~~呵呵;
    这个东西就是因人而异,你可以询问一下原因,如果是因为团队一些约定俗成的规定或者一些规范性问题,那没问题,以后写的时候注意一下,但是如果是针对个人了,那推荐辞职走人吧。
    edoras
        129
    edoras  
       Jul 1, 2018 via Android
    有些命名我原先也觉得其实挺没必要,但是看了 clean code 后感觉有些变量 self explain 这样真的再好不过,毕竟后来维护的人不一定懂得。如果非要质疑建议你去商量在哪维护一个规范,这样别人写代码时也能遵守,不会出现一半人用这种 一半人用那种的情况。
    koalli
        130
    koalli  
       Jul 4, 2018
    setVisible 应该是 C++或者其他绑定来的接口吧,你直接调用的话提高了耦合,如果将来底层改动的话,就得全局搜索 setVisible 去修改,这里改成统一调用 show 接口我认为是解耦合的做法。同时如果你们的 lua 有自动化测试的话,你直接调用 setVisible 就完蛋了,但是如果你调用统一个 show 接口的话,只需要简单的提供一个测试用的接口就可以搞定了,不知道这么说你能不能 get 到我的点。

    变量名的缩写改动我看着很像是为了减少数据传输才做的。

    其实你如果有疑问可以直接问问你的 leader 为什么这么做,看得出来他的习惯很好,而且能有耐心一点点看你代码给你改动的 leader 真的不多。
    Noriko
        131
    Noriko  
       Jul 5, 2018
    initSkinImage => initSkin
    a.do1() a.do2() => a.do1().do2() (这个要看场景适不适合 fluent 模式)
    updateUserRes => updateUserResponse

    这 2 个或 3 个改后好点,其它的一般般,甚至还不如不改。


    说真的,拿任何一个人的代码给别人看,哪怕他写得再好,总有人觉得不符合自己风格,认为按自己改才更好。这世界上没有什么喜好是大家都完全一致认同的。

    你的 leader 负责是负责,但就行为本身没有任何意义。

    team 代码规约新人进来后讲下,大家保证重要原则必须遵守,小细节也能包容个人习惯。

    如果你 leader 改的是你的代码设计,让你代码效率更高,重构后结构更清晰,这种改动必须要感谢;但如果只是改些变量命名(当然前提是你的变量命名不要太差、太随意)、耍些小 tricky,我只会觉得大概率是因为这种 leader 控制欲太强。因为有些 leader 只要 review 不管代码好坏,他不改点代码就觉得浑身不舒服。。。
    1  2  
    About   ·   Help   ·   Advertise   ·   Blog   ·   API   ·   FAQ   ·   Solana   ·   3215 Online   Highest 6679   ·     Select Language
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.5 · 137ms · UTC 03:33 · PVG 11:33 · LAX 20:33 · JFK 23:33
    ♥ Do have faith in what you're doing.