如何委婉的和新同事说一些代码上的优化建议

V2EX = way to explore

V2EX 是一个关于分享和探索的地方

已注册用户请登录

这是一个创建于 3034 天前的主题，其中的信息可能已经有所发展或是发生改变。

公司有 CodeView 功能, 看到新同事的代码逻辑上没有什么问题，就是有几点小问题

1. 没有遵循 PEP8 代码风格
2. 有些功能可能用别的方法会更好的解决，避免了过多的代码量和逻辑判断

遵循 PEP8 这个还好弄，关于第 2 点应该如何说呢？

pep8

代码

代码量

遵循

22 条回复 • 2016-01-06 20:51:12 +08:00

taozle

2016-01-06 12:21:26 +08:00

直接说啊

asj

2016-01-06 12:26:02 +08:00

code review 应该关注代码风格统一和知识分享，在这个场合少提议重新设计。应该关注代码做了什么，而不是谁写了这段代码。否则这种建议很容易被写代码的人视作对他的批评，最终演变成互撕，或者消极参与。
对于优化的建议，不妨换个思路。不是建议“他的”代码应该怎样，而是分享“我觉得”更好的设计知识。下次 reviewer 或者结对的时候给他 show 你的方法。如果真的有先进性，他自然会借鉴。

bobuick

2016-01-06 12:29:36 +08:00

lz 我也有这样的问题，可是我的问题是如何给老同事说，日了狗了，用的方式，写的代码那么难看就没想到去改一下

jiongxiaobu

2016-01-06 12:56:49 +08:00 via Android

直接说啊

k9982874

2016-01-06 12:58:59 +08:00

你是 review 的负责人么？监督代码优化是你的职责么？
如果是，不用多想这是你的工作。如果不是给自己找点事干吧，太闲了。

GNiux

2016-01-06 12:59:39 +08:00 via iPhone

2 楼正点。赞

clino

2016-01-06 13:10:11 +08:00 via Android

不明白为什么要委婉工程师不是应该直接了当的讨论技术问题吗

GNiux

2016-01-06 13:13:43 +08:00 via iPhone

@clino 他想提风格、效率问题。。

luoway

2016-01-06 13:21:21 +08:00

如果对方用文字记下来了，说明他采纳了，下次发现同样的问题直接说就好。
如果没有用文字记下来，那就说明自己还没有说服对方，下次发现同样的问题还得重试。

推而广之，当面说是头脑风暴，文字交流才是技术分享。

a0000

2016-01-06 13:35:12 +08:00 via Android

直接说比较好

clino

2016-01-06 13:38:10 +08:00

@GNiux 我们做 code review 的时候什么都可以说啊包括风格效率这些
当然不是每个建议都会被采纳不过明显好的做法一般都会采用的

zjuster

2016-01-06 13:38:19 +08:00

直接说 ok 的。但是千万不要用装逼的语气就好啦。这种沟通需要技巧。

@clino 说服别人很难。

raincious

2016-01-06 13:39:56 +08:00

轻轻的走过去，拍他的肩膀，温柔的说：

小 X ，：
1 、如果你以后写代码遵守 PEP8 规则的话，我就请你吃烧烤。
2 、如果你以后多做做设计而不是用 if 解决问题的话，我就请你吃烧烤。

GNiux

2016-01-06 13:41:56 +08:00 via iPhone

@clino 不过非技术问题就在于你认为“好”的，不是他认为的呢

无解;)

Potter

2016-01-06 13:42:38 +08:00

@raincious 哈哈，烧烤爱好者，你好

ShadowFiend

2016-01-06 13:43:46 +08:00

@clino 因为有一次在 review 时写了一些建议，但是在后面的时候对方没有去修改，所以不知道是否再去写一些建议

例子： django model
date_created = models.DateTimeField()
我建议可以这样
date_created = models.DateTimeField(auto_now_add=True)
说明了下，发现对方没有采纳

ShadowFiend

2016-01-06 13:45:31 +08:00

@bobuick 哈哈我是想更好的同事相处，所以了解下如何最好

ShadowFiend

2016-01-06 13:49:20 +08:00

@k9982874 算是负责人，那如果功能上没有问题呢，这时候的建议是否有更好的表达方式

shibo501c

2016-01-06 14:03:45 +08:00

用 lint

clino

2016-01-06 14:23:26 +08:00

@ShadowFiend code review 除了写下来,最好当面或者电话沟通说一遍
我们用 gerrit,一般来说 patch 都是别人看过没问题以后 merge 的,有时候会有改过很多次如 10 次以上才 merge 的情况

paw

2016-01-06 16:39:48 +08:00

制定公司代码规范。。。

Lpl

2016-01-06 20:51:12 +08:00 via Android

要是我肯定很愿意让你说啊。。。有 code review 都是好公司啊，我代码写的烂自己知道但是不知道咋改，也没人告诉我。。要是我，你说多了我还请你吃饭