u***8 发帖数: 1581 | 1 要检测日期,2015-10-11 是不是YYYY-MM-DD的格式。
if (!startDate.matches("\d{4}-\d{2}-\d{2}")) {
throw new Exception("ERROR: startDate " + startDate + " is
invalid.");
}
reviewer说,Please compile the pattern + matcher, and use the matcher to
match the dates.
我问,值得么?
他说。In terms of performance, yes.
这个真的很大的performance 的关系么?
还有,return new X>(new Y (Y)
非要分段写, 3行。
说是Readability.
这算鸡蛋里面挑骨头么?
更新: 3楼之后。
http://stackoverflow.com/questions/2149680/regex-date-format-va
这个检测我是看这个的。
第2条, 我看code里面很多都是如此写的。 不用第二次的local variable就省在new里
面不可以么? |
j******o 发帖数: 4219 | |
f*******t 发帖数: 7549 | 3 第一条是必须的。
第二条你们应该先反省为啥会写出这种垃圾代码 |
t******4 发帖数: 134 | 4 不算,改不了太多撒
如果每次给你抓一点错就有挑刺嫌疑,一次把问题说完,改好就approve就不算
[在 uno88 (Ut) 的大作中提到:]
:要检测日期,2015-10-11 是不是YYYY-MM-DD的格式。
:
:........... |
u***8 发帖数: 1581 | 5 但是这家伙就是明显挑刺,写很多不痛不痒的。
组要downsize,manager就要他们几个code review给我狂写。
【在 t******4 的大作中提到】 : 不算,改不了太多撒 : 如果每次给你抓一点错就有挑刺嫌疑,一次把问题说完,改好就approve就不算 : [在 uno88 (Ut) 的大作中提到:] : :要检测日期,2015-10-11 是不是YYYY-MM-DD的格式。 : : : :...........
|
j******o 发帖数: 4219 | 6 第一个代码是bug,你转的帖子里说的很清楚了,和performance无关。
第二个就是你们公司的规范的问题了,你要保证随便一个新来的人能看懂你的代码。公
司需要的是工作能够顺利交接,而不是你的code写的有多fancy。 |
n*******1 发帖数: 145 | |
c******f 发帖数: 243 | 8 不算挑刺
我做CR的话,我会加上"\d{4}-\d{2}-\d{2}"为什么不是constant |
u***8 发帖数: 1581 | 9 team要砍掉,要去别的组分流。
这几个coder被manager威胁,要给我bad review,好开我走人,不给package。
有个哥们给我说了,但是他不做这种事情。
【在 u***8 的大作中提到】 : 要检测日期,2015-10-11 是不是YYYY-MM-DD的格式。 : if (!startDate.matches("\d{4}-\d{2}-\d{2}")) { : throw new Exception("ERROR: startDate " + startDate + " is : invalid."); : } : reviewer说,Please compile the pattern + matcher, and use the matcher to : match the dates. : 我问,值得么? : 他说。In terms of performance, yes. : 这个真的很大的performance 的关系么?
|
s*****r 发帖数: 43070 | 10 1 yes
2 如果constructor很复杂,可以用builder pattern,比较清楚 |
|
|
p**********g 发帖数: 187 | 11 第一个代码根本不对啊,此模式不能查出所有非法日期。建议用joda |
j**********r 发帖数: 3798 | 12 1. Not needed unless it's in a loop or in critical path. But you can't say
reviewer is picky.
2. Point to Java 8 streaming and ask them to fuck off. |
b*******y 发帖数: 2048 | 13 算,但是给人打工就是这样了。。。工作要么忍要嘛滚 |
u***8 发帖数: 1581 | 14 谢谢。
还有
for (String line; (line = reader.readLine()) != null;) {
他说要
Use a while loop as is standard. Or use the IOUtils to read fully.
这算么?
【在 b*******y 的大作中提到】 : 算,但是给人打工就是这样了。。。工作要么忍要嘛滚
|
p**r 发帖数: 5853 | 15 哥们,遇到这种情况,速度招下家吧。
头铁了心要你走,你早晚都得走。
之前我头找我茬的时候,
我就直接和头说,你要只是不满意我骂人,我以后就不骂人
你要不想我这里呆着了,那你直说,我就去找工作了。
给个痛快话,大家以后还是朋友。
【在 u***8 的大作中提到】 : team要砍掉,要去别的组分流。 : 这几个coder被manager威胁,要给我bad review,好开我走人,不给package。 : 有个哥们给我说了,但是他不做这种事情。
|
j*****8 发帖数: 3635 | 16 那你头怎么回答你的?
【在 p**r 的大作中提到】 : 哥们,遇到这种情况,速度招下家吧。 : 头铁了心要你走,你早晚都得走。 : 之前我头找我茬的时候, : 我就直接和头说,你要只是不满意我骂人,我以后就不骂人 : 你要不想我这里呆着了,那你直说,我就去找工作了。 : 给个痛快话,大家以后还是朋友。
|
p**r 发帖数: 5853 | 17 头说,如果你不骂人,你就是完美人才。
然后俺就混混日子刷刷买买提,看着FLG的贫困线流口水,
一年又过去了,唉,俺的青春啊。。。
【在 j*****8 的大作中提到】 : 那你头怎么回答你的?
|
b**********5 发帖数: 7881 | 18 this is correct
【在 p**********g 的大作中提到】 : 第一个代码根本不对啊,此模式不能查出所有非法日期。建议用joda
|
h**********I 发帖数: 51 | 19 是不是挑刺每个人的感觉也不一样的,因为主观标准不一样。
以上几点我觉得都不是在挑刺。
尤其是这个:“for (String line; (line = reader.readLine()) != null;) {”
如果是我的话我会给出同样的review。 |
j******o 发帖数: 4219 | 20 看对方的目的了,如果只是纠正代码那正常。如果是要以此赶你走的话就算挑刺了,不
过你也没什么办法。 |
|
|
e**********0 发帖数: 502 | 21
不算
撇开你老板要不要开人的政治上原因,这两行代码我们组code review都会被打回来
我也看到好多其他组根本就不在乎类似的,却绝于每个组leader对coding的态度
【在 u***8 的大作中提到】 : 要检测日期,2015-10-11 是不是YYYY-MM-DD的格式。 : if (!startDate.matches("\d{4}-\d{2}-\d{2}")) { : throw new Exception("ERROR: startDate " + startDate + " is : invalid."); : } : reviewer说,Please compile the pattern + matcher, and use the matcher to : match the dates. : 我问,值得么? : 他说。In terms of performance, yes. : 这个真的很大的performance 的关系么?
|
x*******1 发帖数: 28835 | |
x*******1 发帖数: 28835 | |
t********c 发帖数: 28 | 24 我这种readability都是给人敲章水过的人也忍不住喷你一下你的代码了
三个点都该喷
真正鸡蛋挑骨头的你没见过
变量命名要改,class名要改,改了之后 突然又觉得另外一个名字更好,继续改
代码结构要改,但又讲不出道理,只是是说他觉得那样更好,哼哧哼哧搞了一两天之后
,我发现此路不通,再费半个小时口舌告诉他为什么不能这么改, |
u***8 发帖数: 1581 | 25 此人原来做过类似的review给我。
结果自己打脸。
【在 t********c 的大作中提到】 : 我这种readability都是给人敲章水过的人也忍不住喷你一下你的代码了 : 三个点都该喷 : 真正鸡蛋挑骨头的你没见过 : 变量命名要改,class名要改,改了之后 突然又觉得另外一个名字更好,继续改 : 代码结构要改,但又讲不出道理,只是是说他觉得那样更好,哼哧哼哧搞了一两天之后 : ,我发现此路不通,再费半个小时口舌告诉他为什么不能这么改,
|
y*d 发帖数: 2226 | 26 就事论事的话,他提这两点都有道理
如果是检查输入的话,一般来讲,performance根本就不是个事。但是,用regular
expression的时候先compile出个pattern再用是个好习惯
第二条,你那个代码确实难看
不过都不是什么大不了的问题。在代码质量要求高的地方,确实都需要改了。如果不是
什么特别强调代码质量的地方,这么写也可以了。 |
f*****n 发帖数: 2126 | 27 问题就是,之前都这么写啊。现在要裁人了,这斯就开始叽歪了。
【在 y*d 的大作中提到】 : 就事论事的话,他提这两点都有道理 : 如果是检查输入的话,一般来讲,performance根本就不是个事。但是,用regular : expression的时候先compile出个pattern再用是个好习惯 : 第二条,你那个代码确实难看 : 不过都不是什么大不了的问题。在代码质量要求高的地方,确实都需要改了。如果不是 : 什么特别强调代码质量的地方,这么写也可以了。
|
y*d 发帖数: 2226 | 28
为这么点鸡毛蒜皮的小问题,扯到裁员就太扯了
但是裁员这事吧,就好像毛主席抓右派。上面定了名额,要抓出1/3的右派出来,他不
把你搞成右派,他自己就有成右派的危险。把你抓了,他就安全了
【在 f*****n 的大作中提到】 : 问题就是,之前都这么写啊。现在要裁人了,这斯就开始叽歪了。
|
d******k 发帖数: 4295 | 29 如果你的matches是在一个loop里头,在外面compile the pattern很提高速度。
【在 u***8 的大作中提到】 : 要检测日期,2015-10-11 是不是YYYY-MM-DD的格式。 : if (!startDate.matches("\d{4}-\d{2}-\d{2}")) { : throw new Exception("ERROR: startDate " + startDate + " is : invalid."); : } : reviewer说,Please compile the pattern + matcher, and use the matcher to : match the dates. : 我问,值得么? : 他说。In terms of performance, yes. : 这个真的很大的performance 的关系么?
|
u***8 发帖数: 1581 | 30 问题就是不在loop里面。
【在 d******k 的大作中提到】 : 如果你的matches是在一个loop里头,在外面compile the pattern很提高速度。
|
|
|
u***8 发帖数: 1581 | 31 改变量名,这个reviewer之前也做过类似的事情哦。
【在 t********c 的大作中提到】 : 我这种readability都是给人敲章水过的人也忍不住喷你一下你的代码了 : 三个点都该喷 : 真正鸡蛋挑骨头的你没见过 : 变量命名要改,class名要改,改了之后 突然又觉得另外一个名字更好,继续改 : 代码结构要改,但又讲不出道理,只是是说他觉得那样更好,哼哧哼哧搞了一两天之后 : ,我发现此路不通,再费半个小时口舌告诉他为什么不能这么改,
|
u***8 发帖数: 1581 | 32 对的。我说的就是这么回事。
【在 f*****n 的大作中提到】 : 问题就是,之前都这么写啊。现在要裁人了,这斯就开始叽歪了。
|
u***8 发帖数: 1581 | 33 但是裁员这事吧,就好像毛主席抓右派。上面定了名额,要抓出1/3的右派出来,他不
真相。 |
m****u 发帖数: 3915 | 34 这算个p的挑刺
都是很正常的建议,楼主这么defensive怪不得你头不爽你 |
s******1 发帖数: 404 | 35 总之一句话,最好的代码是本科二年级就能看懂能改的代码。
别玩花,别玩火。 |
s******n 发帖数: 793 | 36 你应该感谢给你写这些review的人
自己出错还嫌弃
【在 u***8 的大作中提到】 : 要检测日期,2015-10-11 是不是YYYY-MM-DD的格式。 : if (!startDate.matches("\d{4}-\d{2}-\d{2}")) { : throw new Exception("ERROR: startDate " + startDate + " is : invalid."); : } : reviewer说,Please compile the pattern + matcher, and use the matcher to : match the dates. : 我问,值得么? : 他说。In terms of performance, yes. : 这个真的很大的performance 的关系么?
|
u***8 发帖数: 1581 | 37 他也没看出来那个不能检查date。
【在 s******n 的大作中提到】 : 你应该感谢给你写这些review的人 : 自己出错还嫌弃
|
S*******w 发帖数: 24236 | 38 第一个条说的很对啊。 performance是个big concern
不过如果你这个method 几秒中或者几分钟才call一次 也就无所谓了。 |