200 简单的代码评审，看看有什么问题

在路上20130607 2012-05-14 10:28:49

就看这样写有什么不好



 System.Diagnostics.Stopwatch watch = new System.Diagnostics.Stopwatch();

                object re = null;

                try

                {

                    re = dbCommand.ExecuteScalar();

                    watch.Stop();

                }

                catch (Exception e)

                {

                    watch.Stop();

                    throw e;

                }

                finally

                {

                    WriteSqlLog(sqlLog, watch.Elapsed.TotalSeconds);

                }

...全文

1198 28 打赏收藏转发到动态举报

写回复

用AI写文章

28 条回复

切换为时间正序

请发表友善的回复…

发表回复

jayrao5566 2012-05-18

打赏
举报

[Quote=引用 3 楼的回复:]

引用 1 楼的回复:

watch.Start();

这个是个问题
[/Quote]
+1

LSP学逆向 2012-05-18

打赏
举报

[Quote=引用 21 楼的回复:]

StopWatch是计时的么。。。

这个帖子让我灵感来了。。等会再存档一篇老帖子。。
[/Quote]

野比。。。。。。

在路上20130607 2012-05-14

打赏
举报

[Quote=引用 13 楼的回复:]

数据库连接关闭了再记录日志.

[/Quote]
这个之前还没注意过谢谢了

在路上20130607 2012-05-14

打赏
举报

[Quote=引用 11 楼的回复:]

单单从代码，你的逻辑，有点过怪。
这个是关闭，watch.Stop(); 代码块是吗
可是你又在关闭的有执行：dbCommand.ExecuteScalar();
你的启动程序又做什么？
另外 throw的时候自定义比较好。
[/Quote]

谢谢你的回复
这段代码逻辑的确不好，对于项目发布后，这中方法是不好的，12楼有解释。

事理 2012-05-14

打赏
举报

[Quote=引用 12 楼的回复:]

下面是同事讨论的一些结果
1. finally 不管怎么样都会执行那么watch.Stop(); 可以写到finally
2.watch 没有start() 这个不是问题考察点
3. try catch throw 会改变堆栈，无法最终出错的是哪行. 这个在分析新疆日志是经常碰到的
4. 可能也不是问题的考察点尽可能捕获自己能处理的异常少用整个Exception
……
[/Quote]

try catch throw 会改变堆栈

这样写就没问题了
try
{}
catch(Exception e)
{
throw;
}

事理 2012-05-14

打赏
举报

记录日志不要放到数据库访问中
据说io操作耗时间,会是数据库连接占用更长.

System.Diagnostics.Stopwatch watch = new System.Diagnostics.Stopwatch();
object re = null;
try
{
re = dbCommand.ExecuteScalar();
}
catch(SqlException e)
{
throw e;
}
catch (Exception e)
{
throw e;
}
finally
{
watch.Stop();

//WriteSqlLog(sqlLog, watch.Elapsed.TotalSeconds);数据库连接关闭了再记录日志.
}

在路上20130607 2012-05-14

打赏
举报

下面是同事讨论的一些结果
1. finally 不管怎么样都会执行那么watch.Stop(); 可以写到finally
2.watch 没有start() 这个不是问题考察点
3. try catch throw 会改变堆栈，无法最终出错的是哪行. 这个在分析新疆日志是经常碰到的
4. 可能也不是问题的考察点尽可能捕获自己能处理的异常少用整个Exception

段传涛 2012-05-14

打赏
举报

单单从代码，你的逻辑，有点过怪。
这个是关闭，watch.Stop(); 代码块是吗
可是你又在关闭的有执行：dbCommand.ExecuteScalar();
你的启动程序又做什么？
另外 throw的时候自定义比较好。

ilooknow 2012-05-14

打赏
举报

System.Diagnostics.Stopwatch watch = new System.Diagnostics.Stopwatch();
object re = null;
try
{
watch.Start();
re = dbCommand.ExecuteScalar();
watch.Stop();
}
catch (Exception e)
{
watch.Stop();
throw e;
}
finally
{
WriteSqlLog(sqlLog, watch.ElapsedMilliseconds);
}

铜臂阿铁木 2012-05-14

打赏
举报

watch.Elapsed.TotalSeconds

这个单位有点大。
最好使用ms，如果你的操作有很长的有很短的，时间在10ms-10minutes的区间，你最好写个方法，自动转换单位。
比如
public string FormatCostTime(Stopwatch currentWatch){
long ms = currentWatch.Elapsed.TotalMilliseconds ;
if(10*1000*1000> ms && ms > 1000 * 1000)
{
return string.Format("{0}m",currentWatch.Elapsed.TotalMinutes);
}
else if(1000<ms &&　ms<=1000*1000)
{
return string.Format("{0}s",currentWatch.Elapsed.TotalSeconds);
}
else if(ms<=1000)
{
return string.Format("{0}ms",currentWatch.Elapsed.TotalMilliseconds );
}
else
{
return string.Format("{0}h",currentWatch.Elapsed.TotalHours);
}

}

在路上20130607 2012-05-14

打赏
举报

别人有没有什么看法自己顶一下·········

在路上20130607 2012-05-14

打赏
举报

[Quote=引用 5 楼的回复:]

如期记录日志是不明确的，不知道程序运行成功与否
[/Quote]
记录了执行的sql 没有记录是不是正确执行了··

在路上20130607 2012-05-14

打赏
举报

[Quote=引用 4 楼的回复:]

1.watch.start();
2.注释
3.对象回收
4.这段代码是在主程序中还是在自定义的方法里面，如果实在主程序中则throw e这个句子就不能用吧。
[/Quote]
2：的确没有
3：回收Stopwatch ？
4：我也认同在chtch中写 throw e 不好