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

Question

就看这样写有什么不好 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);
                }

Answer 1

watch.Stop();可以放到finally里面，不过如果是为了精确得出TotalSeconds，貌似就应该这样。
dbCommand这个数据库连接后面还有用到吗，没用到要关闭。

Answer 2

1.watch.start();
2.注释
3.对象回收
4.这段代码是在主程序中还是在自定义的方法里面，如果实在主程序中则throw e这个句子就不能用吧。

Answer 3

2：的确没有 
3：回收Stopwatch ？
4：我也认同在chtch中写 throw e 不好

Answer 4

记录了执行的sql 没有记录是不是正确执行了··

Answer 5

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);
   }}

Answer 6

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);
                }

Answer 7

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

Answer 8

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

Answer 9

记录日志不要放到数据库访问中
据说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);数据库连接关闭了再记录日志.
                }

Answer 10

try catch throw 会改变堆栈这样写就没问题了
try
{}
catch(Exception e)
{
    throw; 
}

Answer 11

这才是关键，其它的诸如忘了 start 之类的，都是小问题，程序跑一边就会知道的。也可能是楼主忘记写了。
stop 放的位置虽说重复，但与楼主问题的关键不太贴切。

Answer 12

watch.Stop();应该放在  finally里前面的都不要。

Answer 13

同意4，但是3说的不准确，throw e会向上层抛出异常，在上层逻辑中可以使用exp.InnerException.InnerException来逐层捕获内部异常，直接最底层就可以知道原凶了

Answer 14

StopWatch是计时的么这个帖子让我灵感来了。。等会再存档一篇老帖子。。

Answer 15

watch.Stop(); 应该写到finally里