同事写代码很随意,该如何应对?

pate 发布于 2017年10月21日 | 更新于 4月前
tinyfool 等1人欣赏。

我所在的开发小组共三人,我担任组长。

同事A,年纪比我大,思维不严谨,代码很多错漏,共事差不多三年了。

同事B,大概一年多的开发经验,今年才加进来的。他实习的时候适应得很快,我感觉终于有个潜力新人可以分担我的压力了,但实习完一起工作一段时间后我觉得无望了。我列举最主要的两件事。

第一件事,他写了一个函数,5个参数中有两个是没有用到的。我跟他说这样写不合理,问他为什么会这样 写,他解释一开始是打算xxx所以要用到那两个参数,后来又决定xxx所以没用到这两个参数。我跟他说完之后就去了洗手间,回来问他删了那两个参数没有,他说“没有,这又不影响功能”(大概意思,非原话),在我要求他必须删除的时候他才删除,我从他的表情语气感觉他心里还不服气。

第二件事,原来有一个函数doSomething(po, callback),是处理一条数据的,后来要加批量处理的功能,他直接把函数功能扩展成了doSomething(po, type, callback),调用函数的代码是这样写的:

var po = {xxx};
po.ids = "1,2,3";
doSomething(po, 2, callback);

我看到后真的要吐血了,用即时通讯工具跟他说这样写非常莫名其妙,他连一句话回应都没有。


2017年10月29日补充说明:

关于第二件事,我用“莫名其妙”这个词应该是不妥的。我这里解释一下当时为什么会这么说。

第一,这里的功能场景是这样的,浏览器展示一个数据列表,用户可对某条数据操作,也可选中多条数据批量操作。单条数据操作时,需要展示该条数据的详细信息,批量操作则不需要展示数据的详细信息。

第二,说说更详细的函数说明及调用函数方式。

/**
 * xxx(函数功能描述)
 * @param po 会计凭证
 * @param type 类型(1-单条数据处理;2-批量数据处理)
 * @param callback 回调函数
 */
function doSomething(po, type, callback) {
    xxx
}

调用函数进行批量数据处理的时候,他首先是获取列表的第一条数据,得到一个数据的对象po,然后获取用户选择的N条数据的ID组装成字符串ids,然后把字符串ids放到po中。

var po = grid.getRowData(0);
var ids = grid.getSelectIds();
po.ids = ids;
doSomething(po, 2, function() {xxx});

首先,我看函数的参数和参数说明完全不明白怎么能实现批量操作,然后调用的时候做了一个动作“取列表的第一条数据”这是与批量操作完全不相干的事情。

如果是我写这个函数的话,会是这样的:

/**
 * xxx
 * @param operationType 操作类型(1-单条数据处理;2-批量数据处理)
 * @param po 凭证数据(操作类型为单条数据时使用,批量数据处理时为null)
 * @param ids 批量数据处理的数据ID字符串,格式:“23,24,25”
 * @param callback 处理成功后的回调函数
 */
function doSomething(opertionType, po, ids, callback) {
    xxx
}
共14条回复
tinyfool 回复于 2017年10月22日

代码质量这个事儿,在项目小,或者没有经验的前提下,没有感受,也倒不是很奇怪。

我觉得你不能只给他讲一个结果,要给他讲,他那么做会有什么样的后果,而不是说,你错了,你改,那么他可能接受不了吧

easyfly 回复于 2017年10月22日

说句不中听的话,你的这种方式我非常不认同,简直就在挑起部门同事之间的矛盾。

非常讨厌有个人在我写代码的时候指手画脚,说这个不该这么写,那个必须改。也许他是对的,然后呢,嫌弃我写的不好,不如你自己来呀。要是哪天被我发现你在代码中的bug,我会千百倍怼回去。

如果你觉得他们的代码质量已经到了必须要提醒注意的地步,那么最好的方式是建立比较规范的代码review 制度,比如必须在经过你的或者其他人的 review 之后才可以提交,这样的话,你要挑刺,也显得更加对事不对人。

或者你既然是组长,那么是否可以在每周召开一次代码回顾会议,来回看一下这周的代码,分析一些写的不好的地方(注意不仅要回看这两个人的不好的代码,最好把你的自己的写的不好的代码也放上去),这样更加能够让人接受。

要把注意力放在如何帮助他们提高水平,写出更好的代码上,而不是去挑他们的错上 :)

qiezi 回复于 2017年10月22日

因为你没有完整标准,看起来像是在挑刺,这是你在意而他无所谓的。配置好lint和代码自动格式化,检查你说的这种基本问题,并把代码格式化为统一风格,提交前自动执行。大家执行相同的标准,检查不通过就无法提交,还可以放到服务端强制。

其他方面你说烂代码或设计问题,有时候这些只是个人喜好,容易引起争论,强制还没什么实际收益,团队有默契了再进行。

我见过很多项目交接,第一步都是吐槽之前的人写的烂,然后花一个月来重构,时间允许的话就让他们做,正好熟悉需求。但我并不会觉得之前的有多烂,很多时候只是个人习惯而已,另一个团队再接手会觉得之前两个都很烂。配置好CI运行单元测试和集成测试来检验是否完成了需求,这是底线。

看起来你和同事还没达成默契,通讯工具上指出别人问题效果是最差的,而且你还用了莫名其妙这种词,没人能接受吧。

我建议你先以自己做例子,说明加入lint和格式化工具后,纠正了自己很多代码问题(举出具体例子),顺便也发现了别的同事的少量代码问题,我们先试用一下。然后加入进去,慢慢就习惯了,再加入CI,定期分享发现的问题,鼓励大家写一写关键部分的测试(别强求覆盖率)。

xiaoyouke 回复于 2017年10月22日

这个还是你们那儿的管理不行,如果规定代码有一处bug扣多少钱的话他能不改吗?

tinyfool 回复于 2017年10月22日

2楼 @easyfly

3楼 @qiezi

二位说得好有道理

梦中醒不过来 回复于 2017年10月22日

每周做code review算绩效,把这事流程化就行了,不用讲道理

清醒疯子 回复于 2017年10月22日 | 更新于 4月前

我们每个版本,移测就开始评审代码。两周一版本。

PHPReactNative 回复于 2017年10月23日

我们每次Merge都会强制CR

人在江天 回复于 2017年10月23日

就想问下最后说的莫名其妙在何处

tonyliu 回复于 2017年10月24日

感觉你这个问题更多还是在沟通方式上.

第一件事, 喜欢这么写代码的人, 通常对生活也是很随意的, 怎么舒坦怎么来, 生活垃圾也随处可见, 这种时候你可以跟他好好沟通的, 剖析这种利害关系, 告诉他这样的代码风格很容易会导致代码有杂音, 就像我们平时生活中这类的东西多了后会导致很难集中起精神来是一样的. 让他自己去想, 对方如果还坚持, 你可以要求他尽量在跟你一起处事的时候能有一些这种 "洁癖". 现在可能对你会有点意见, 这么坚持下来他会感谢你的.

第二件事你的做法才是莫名其妙的, 你大可以将你自己处理这个问题的思路说出来, 甚至写出你解决这个问题的代码, 然后告诉他你觉得他那么做不好的地方, 你这么做对扩展和后期维护带来的好处. 说出你的顾虑, 让对方也去思考, 这也不会伤到对方, 你这样直接一句 "莫名其妙", 换谁会开心呢?

pate 回复于 2017年10月29日

2楼 @easyfly 我不知道你为什么觉得我是在挑刺,我作为组长,自然有责任维护项目代码的质量,指出代码问题其实也是帮助同事进步。

pate 回复于 2017年10月29日

3楼 @qiezi 谢谢你的建议。

“看起来你和同事还没达成默契,通讯工具上指出别人问题效果是最差的,而且你还用了莫名其妙这种词,没人能接受吧”

1、我和那位同事共事时间不长,沟通不多,确实没有达成默契。

2、用通讯工具沟通是因为在开放办公环境下不宜叫他过来我的工位,我过去他的工位也不方便找出对应的代码。

3、“莫名其妙”这个词在事后看来确实不妥当,可能说“这个函数设计会让人困惑,不易理解”更好。

pate 回复于 2017年10月29日

10楼 @tonyliu

谢谢你的建议。

“第二件事你的做法才是莫名其妙的, 你大可以将你自己处理这个问题的思路说出来, 甚至写出你解决这个问题的代码, 然后告诉他你觉得他那么做不好的地方”

这里我没详细写清楚,我指出这个问题之后,是如你所说把我自己处理这个问题的思路,只是最后说了我对这个函数的第一感觉“莫名其妙”。

当然,用“莫名其妙”这个词确实不好,应该采用“这个函数设计会让人困惑,不易理解”此类说法。

legege007 回复于 4月前

支持楼主的做法,code review和unit test都是很有必要的~

本帖有14个回复,因为您没有注册或者登录本站,所以,只能看到本帖的10条回复。如果想看到全部回复,请注册或者登录本站。

登录 或者 注册
[顶 楼]
|
|
[底 楼]
|
|
[首 页]