首页   注册   登录
V2EX = way to explore
V2EX 是一个关于分享和探索的地方
现在注册
已注册用户请  登录
华为云
V2EX  ›  程序员

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

  •  1
     
  •   fyxtc · 54 天前 · 10160 次点击
    这是一个创建于 54 天前的主题,其中的信息可能已经有所发展或是发生改变。

    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

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

    第 1 条附言  ·  54 天前
    补充一句,好多人说风格问题,可是我真的除了那个 res 的缩写之外(这个是我的错,就算缩写也应该是 resp ),其他的修改完全看不到有任何属于能被风格所定义的修改范畴啊! 又不是什么驼峰大括号 abc 这样的。。。希望你们能先好好看看修改的部分可以吗。。。
    第 2 条附言  ·  53 天前
    谢谢大家的回复,可能是一周内我这个小模块需求就添加了三次(注意是添加),本身自己就算是挺用心的在重构了,所以这些小问题的修改确实影响到了一点自己的情绪。大家说的话我也都看了,我也会选择适合自己的解决方法,谢谢大家
    第 3 条附言  ·  53 天前
    最后一条附言
    首先反驳一下那些说 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 回复  |  直到 2018-07-05 22:33:45 +08:00
    1  2  
        101
    jorneyr   53 天前
    随机显示一些按钮的文本方法名:randSelectLabel => randomOptions

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

    咱不惯着这种 leader

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

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

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

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

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

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

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

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

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

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

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

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

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


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

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

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

    如果你 leader 改的是你的代码设计,让你代码效率更高,重构后结构更清晰,这种改动必须要感谢;但如果只是改些变量命名(当然前提是你的变量命名不要太差、太随意)、耍些小 tricky,我只会觉得大概率是因为这种 leader 控制欲太强。因为有些 leader 只要 review 不管代码好坏,他不改点代码就觉得浑身不舒服。。。
    1  2  
    关于   ·   FAQ   ·   API   ·   我们的愿景   ·   广告投放   ·   感谢   ·   实用小工具   ·   3390 人在线   最高记录 3762   ·  
    创意工作者们的社区
    World is powered by solitude
    VERSION: 3.9.8.1 · 22ms · UTC 06:21 · PVG 14:21 · LAX 23:21 · JFK 02:21
    ♥ Do have faith in what you're doing.
    沪ICP备16043287号-1